Bug 109370 – [chromium] Apply page scale to all WebInputEvent types

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alexandre Elias]

[chromium] Apply page scale to all WebInputEvent types

[Alexandre Elias]

Created attachment 187461 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[James Robinson]

Comment on attachment 187461 [details]
Patch

What about doing these conversions in WebInputEventConversions.cpp?  It already deals with the general problem of turning a WebInputEvent into something that WebCore understands

[WebKit Review Bot]

Comment on attachment 187461 [details]
Patch

Attachment 187461 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16470776

New failing tests:
fast/frames/frame-set-scaling-hit.html
fast/events/touch/gesture/touch-gesture-scroll-div-scaled.html
fast/frames/frame-set-rotation-hit.html
fast/events/page-scaled-mouse-click-iframe.html
fast/events/page-scaled-mouse-click.html
fast/events/touch/page-scaled-touch-gesture-click.html
platform/chromium/plugins/transformed-events.html
fast/events/touch/touch-scaled-scrolled.html

[Alexandre Elias]

Created attachment 187504 [details]
Patch

Move everything to WebInputEventConversions.cpp

[Alexandre Elias]

Good idea, it's much cleaner this way.  I also did the reverse conversion in the PlatformEvent -> WebInputEvent builders.

[James Robinson]

Comment on attachment 187504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187504&action=review

It seems pretty easy to miss a scale or run a scale backwards through here without noticing.  Can you add some cases to WebInputEventConversionTest.cpp to make sure we don't accidentally lose or invert any of the scales you've added?

Why are webkitMovementX/Y unscaled in WebMouseEventBuilder?

> Source/WebKit/chromium/src/WebInputEventConversion.cpp:436
>      webEvent.globalX = event.screenX();
>      webEvent.globalY = event.screenY();

why are these not scaled?

[WebKit Review Bot]

Comment on attachment 187504 [details]
Patch

Attachment 187504 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16492050

New failing tests:
platform/chromium/plugins/transformed-events.html

[Alexandre Elias]

Created attachment 187736 [details]
Patch

Add unit test; add missing reverse-mouse scales; change widget->parent() to widget->root() for robustness; fix disambiguation scaling; revert int->float conversion since plugins reinterpret the bytes as ints

[Alexandre Elias]

OK, this should be ready for landing.

> Why are webkitMovementX/Y unscaled in WebMouseEventBuilder?
> 

That was a mistake; I'm now scaling them.

> > Source/WebKit/chromium/src/WebInputEventConversion.cpp:436
> >      webEvent.globalX = event.screenX();
> >      webEvent.globalY = event.screenY();
> 
> why are these not scaled?

This one is intentional.  According to comments, these refer to location relative to the top-left corner of the physical screen.  It doesn't make sense to apply page scale to this.

[WebKit Review Bot]

Comment on attachment 187736 [details]
Patch

Attachment 187736 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16497497

[James Robinson]

Comment on attachment 187736 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187736&action=review

R=me, one question for your consideration

> Source/WebKit/chromium/src/WebInputEventConversion.cpp:178
> +        m_area = IntSize(e.data.tap.width / scale, e.data.tap.height / scale);

do we want to use an explicit conversion here so we know what sort of rounding we're getting?  Maybe go to a FloatSize first and then use enclosing/enclosed/whateverIntSize?

[James Robinson]

(In reply to comment #11)
> (From update of attachment 187736 [details])
> Attachment 187736 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/16497497

Looks like you need some 'u' suffixes

[WebKit Review Bot]

Comment on attachment 187736 [details]
Patch

Attachment 187736 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16501462

[Alexandre Elias]

Created attachment 187741 [details]
Patch

Switch to expandedIntSize for size conversions; fix signed/unsigned integer warning

[WebKit Review Bot]

Comment on attachment 187741 [details]
Patch

Clearing flags on attachment: 187741

Committed r142571: <http://trac.webkit.org/changeset/142571>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.