Pass correct client coordinates for mouse events synthesized from a GestureTap

Pass correct client coordinates for mouse events synthesized from a GestureTap

After re-adding the extra hit-tests on GestureTap in crrev.com/429793004 the
scroll offset was being added twice to the synthesized mouse events. This went
unnoticed because the target had already been determined in earlier hit-tests
and so the event was dispatch to the proper target but with the wrong
coordinates. This CL fixes that to match the exact coordinates that native
mouse events would report.

BUG=418800
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183699

[Zeeshan Qureshi, 2014-10-09 19:30:03]

zeeshanq@chromium.org changed reviewers:
+ rbyers@chromium.org

[Zeeshan Qureshi, 2014-10-09 19:41:33]

Rick, PTAL.

[Rick Byers, 2014-10-10 00:39:35]

Good work tracking this down, I'm suitably embarrassed!
Just a couple minor suggestions.

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
File LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html
(left):

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html:35:
shouldBe("event.clientY", "50 / 2 + 7");
Ok so it looks like the real problem here is that I was thinking pageY when I
typed clientY - damn.  To help avoid this sort of bug in the future, can you
please also check the pageX/pageY co-ordinates here too?  I.e. you'll need to
actually use the 7 value (maybe call it frameScrollOffset) somewhere (eg. to
verify that the test properly cased the frame to scroll in the first place at
all)...

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
File LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html
(right):

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html:19: var
scrollOffset = 3;
nit: call this windowScrollOffset (to avoid confusion with the frame scroll
offset).

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html:35:
shouldBe("event.clientX", "point.x");
using "point" here seems wrong because point is in window co-ordinate space but
client is in the co-ordinate space of the frame.  We're apparently getting away
with it only because the frame happens to be at (0,0) in the document.

Can you fix this by adding some margin to the target, then change the
expectation to be just the relative co-ordinate.  Eg. maybe:

var pointInFrame = {
  x: rect.width / 2,
  y: rect.height / 2
};
var pointInWindow = {
  x: rect.left + pointInFrame.x,
  y: rect.top + pointInFrame.y
};

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
File LayoutTests/fast/events/touch/gesture/gesture-tap-scrolled.html (right):

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-scrolled.html:23: function
onEventInFrame(e) {
nit: just call this onEvent (onEventInFrame is a special name the iframe
event-delegator looks for - we should avoid potential confusion)

[Zeeshan Qureshi, 2014-10-10 00:47:23]

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
File LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html
(right):

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrolled.html:35:
shouldBe("event.clientX", "point.x");
On 2014/10/10 00:39:35, Rick Byers wrote:
> using "point" here seems wrong because point is in window co-ordinate space
but
> client is in the co-ordinate space of the frame.  We're apparently getting
away
> with it only because the frame happens to be at (0,0) in the document.
> 
> Can you fix this by adding some margin to the target, then change the
> expectation to be just the relative co-ordinate.  Eg. maybe:
> 
> var pointInFrame = {
>   x: rect.width / 2,
>   y: rect.height / 2
> };
> var pointInWindow = {
>   x: rect.left + pointInFrame.x,
>   y: rect.top + pointInFrame.y
> };

That looks more readable, will update.

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
File LayoutTests/fast/events/touch/gesture/gesture-tap-scrolled.html (right):

https://codereview.chromium.org/641063003/diff/150001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-scrolled.html:23: function
onEventInFrame(e) {
On 2014/10/10 00:39:35, Rick Byers wrote:
> nit: just call this onEvent (onEventInFrame is a special name the iframe
> event-delegator looks for - we should avoid potential confusion)

oops, missed updating this.

[Zeeshan Qureshi, 2014-10-14 19:51:21]

Patchset #4 (id:220001) has been deleted

[Zeeshan Qureshi, 2014-10-14 19:56:16]

Rick, tried to make the tests more clear, PTAL.

[Rick Byers, 2014-10-14 20:55:01]

Thanks, these tests are substantially stronger (and easier to understand now)! 
LGTM

[Zeeshan Qureshi, 2014-10-14 20:55:48]

The CQ bit was checked by zeeshanq@chromium.org

[commit-bot: I haz the power, 2014-10-14 20:56:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/641063003/260001

[commit-bot: I haz the power, 2014-10-14 22:01:18]

Committed patchset #5 (id:260001) as 183699