Bug 60916 - Plug-in hit testing is broken after zooming
Summary: Plug-in hit testing is broken after zooming
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Unspecified
: P2 Normal
Assignee: Chris Marrin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-16 13:32 PDT by Chris Marrin
Modified: 2011-05-16 15:32 PDT (History)
0 users

See Also:


Attachments
Patch (5.15 KB, patch)
2011-05-16 13:50 PDT, Chris Marrin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2011-05-16 13:32:21 PDT
After zooming, events on plug-ins have the wrong coordinates, which means that plug-ins don't hit-test correctly.

You can see this on the video on nytimes.com
Comment 1 Chris Marrin 2011-05-16 13:50:42 PDT
Created attachment 93688 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-05-16 14:04:53 PDT
Comment on attachment 93688 [details]
Patch

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

Would be nice to have a testcase, which should be possible with the test plugin and CSS transforms if the code were more general.

What about WebKit1?

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:579
> +        float scaleFactor = frame()->pageScaleFactor();
> +        WebMouseEvent webMouseEvent(static_cast<const WebMouseEvent*>(currentEvent), scaleFactor);

It would be nice to make this work in more general way which worked with CSS transforms.

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:648
> +    // Adjust bounds to account for pageScaleFactor
> +    float scaleFactor = frame()->pageScaleFactor();
> +    frameRectInWindowCoordinates.setX(frameRectInWindowCoordinates.x() / scaleFactor);
> +    frameRectInWindowCoordinates.setY(frameRectInWindowCoordinates.y() / scaleFactor);

Ditto.
Comment 3 Darin Adler 2011-05-16 14:05:16 PDT
Comment on attachment 93688 [details]
Patch

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

r=me

> Source/WebKit2/Shared/WebMouseEvent.cpp:81
> +WebMouseEvent::WebMouseEvent(const WebMouseEvent* event, float pageScaleFactor)

I suggest using a reference rather than a pointer for the event type.

> Source/WebKit2/Shared/WebMouseEvent.cpp:85
> +    , m_globalPosition(m_position)

I believe you can write:

    , m_globalPosition(m_position + (event->globalPosition() - event->position()))

And then avoid the code below that calls the move function.

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:579
> +        WebMouseEvent webMouseEvent(static_cast<const WebMouseEvent*>(currentEvent), scaleFactor);

I think this local needs a better name. The current event is also a web mouse event. How about eventWithScaledCoordinates?
Comment 4 Chris Marrin 2011-05-16 15:32:42 PDT
Committed r86615: <http://trac.webkit.org/changeset/86615>