Bug 23557 – Allow hit testing of page content without clipping to visible

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Adam Treat]

Currently all hit testing clips the test to the visible content of the frame.  QWebFrame::hitTestContent should not clip to the visible content, rather it should return the hit test result regardless of the currently visible viewport.  This patch adds a simple option that allows performing hit tests without clipping it to the visible viewport.

[Adam Treat]

Created attachment 27053 [details]
Don't clip to visible option

Implements the no clip option for hit tests.

[Nikolas Zimmermann]

Comment on attachment 27053 [details]
Don't clip to visible option

Patch looks fine, despite the fact that HitTestRequest is so ugly - it should really use enums as flags, instead of a wild series of booleans.
It would be great to get that fixed - but that's out of the scope for this patch. r=me.

Would you file a bug on HitTestRequest uglyness?

[Adam Treat]

Are you thinking of an enum that can be OR'd together to generate the appropriate HitTestRequest?  If so I'll come up with a patch... :)

[Adam Treat]

Reopened for layout test.

[Adam Treat]

Created attachment 27072 [details]
Fixes bug and also adds test

Added a test to qwebframe to guard against regression.  I don't see a clean way of adding hit testing layout tests, but if anyone can think of a good way let me know.

[Adam Treat]

Add Simon

[Adam Treat]

Add David Hyatt.

[Nikolas Zimmermann]

(In reply to comment #3)
> Are you thinking of an enum that can be OR'd together to generate the
> appropriate HitTestRequest?  If so I'll come up with a patch... :)
> 
Yes, exactly.

[Simon Fraser (smfr)]

Would using elementFromPoint() be a better way to achieve what you are trying to do here? Maybe it clips to the viewport, and should not? At least it might give you a way to write a LayoutTest without adding code to tst_qwebframe.

[Adam Treat]

We could certainly modify elementFromPoint to use a HitTestRequest that does not clip to the viewport and thereby achieve a LayoutTest using JS, however this is a significant behavioral change that I was not sure is correct.

The purpose of my patch is to make QWebFrame::hitTestContent method work correctly given its name and purpose.

If you like I can modify 'elementFromPoint' accordingly, but are you sure this behavior change is correct and won't break JS DOM API standards?

[Simon Fraser (smfr)]

Actually elementFromPoint takes "client" (viewport) coords, so only finds things that are visible. Darn.

Please update the patch to include a change to the WebCore.base.exp file for Mac, like:

__ZN7WebCore12EventHandler14scrollOverflowENS_15ScrollDirectionENS_17ScrollGranu
  __ZN7WebCore12EventHandler20handleTextInputEventERKNS_6StringEPNS_5EventEbb
-__ZN7WebCore12EventHandler20hitTestResultAtPointERKNS_8IntPointEb
+__ZN7WebCore12EventHandler20hitTestResultAtPointERKNS_8IntPointEbb
 __ZN7WebCore12EventHandler20sendContextMenuEventERKNS_18PlatformMouseEventE

and file another bug for the 'bool' cleanup.

[Darin Adler]

(In reply to comment #5)
> I don't see a clean way
> of adding hit testing layout tests, but if anyone can think of a good way let
> me know.

Generally you can make hit testing layout tests with eventSender, ensuring the correct element is hit. But perhaps you mean something different. This may be a fix that only affects some specific Qt API, not the behavior of the browser? I'm not really clear here on what the issue is.

[Adam Treat]

Darin, you have it right.  The Qt API exposes a hitTestContent method that should return a valid HitTestResult regardless of the current viewport.  As such, this patch is intended to fix the behavior of this one specific Qt API method.

[Adam Treat]

Created attachment 27075 [details]
Previous patches + mac export

Add the mac export line from Simon although I have no way to test if this is correct.

[Adam Treat]

Committed with r40311.

I'll open a bug report and make a patch for the bools next.