Bug 96541 – REGRESSION: hit test doesn't take iframe scroll position into account

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Rick Byers]

On current chromium builds, we're seeing EventHandler::hitTestResultAtPoint get the wrong location when inside an iframe that is scrolled (behaves as if the iframe scroll position was at the top).

I suspect r127457 <http://trac.webkit.org/changeset/127457>, testing that now and working on a layout test.

[Rick Byers]

Yep, works fine if I revert r127457.  I'll upload a layout test that demonstrates the issue.  Allen, do you have time to take this?  We're branching for Chrome M23 this weekend, so we'd like a fix (or revert) this week.

[Allan Sandfeld Jensen]

That makes sense. The new code does not adjust for the scrollOffset in FrameView(). It should be simple to fix though by adding frameView()->scrollOffset() to the point passed to the child frame.

It would be nice also add a test for it.

[Allan Sandfeld Jensen]

(In reply to comment #1)
> Yep, works fine if I revert r127457.  I'll upload a layout test that demonstrates the issue.  Allen, do you have time to take this?  We're branching for Chrome M23 this weekend, so we'd like a fix (or revert) this week.

I will fix it tomorrow if that is okay with you?

[Rick Byers]

Thanks Allan.  It looks simple enough, I'll take a crack at uploading a fix for you to review.  If I don't have the CL in good enough shape tomorrow, then given the urgency, I'll gladly hand it off to you.

[Rick Byers]

Created attachment 163722 [details]
Patch

[Rick Byers]

Looks like I'm not going to get this done today - I'm a little confused about the co-ordinate systems and the right place to apply the scroll offset, will need to dig deeper to understand it fully.  So I'll just reassign to Allan.

I've included a layout test that demonstrates the problem (I've verified it fails in ToT, but passes with r127457 reverted).

[Allan Sandfeld Jensen]

Created attachment 163816 [details]
Patch

[WebKit Review Bot]

Comment on attachment 163816 [details]
Patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html
editing/selection/drag-in-iframe.html
http/tests/local/drag-over-remote-content.html
http/tests/misc/policy-delegate-called-twice.html
http/tests/misc/redirect-to-external-url.html
touchadjustment/scroll-delegation/iframe-with-mainframe-scroll-offset.html
fast/files/null-origin-string.html
fast/dom/nodesFromRect/nodesFromRect-child-frame-content.html
touchadjustment/iframe-boundary.html
editing/pasteboard/drag-drop-dead-frame.html

[Allan Sandfeld Jensen]

Created attachment 163834 [details]
Patch

Do not use FrameView locations since it is relative to parent frameview, instead calculate the location based on border and padding.

[Rick Byers]

Thanks Allen.  I've verified this patch fixes the original issue as well.

[WebKit Review Bot]

Comment on attachment 163834 [details]
Patch

Clearing flags on attachment: 163834

Committed r128462: <http://trac.webkit.org/changeset/128462>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Julien Chaffraix]

Please include the revision that regressed in the bug title next time. That helps find out what a change broke without having to do trips to bugzilla.