Bug 15162 – hit testing does not respect clip paths

• Component: SVG
• Status: RESOLVED FIXED

[jay]

in the attachment, only the visible area should be active, not the whole anchor.

also the cursor remains an arrow, should change to hand.

[jay]

Created attachment 16228 [details]
clip-path with anchor demo

[Eric Seidel (no email)]

Both are hit testing issues (one is not seeming to respect clip-paths during hit testing) but probably should be separate bugs.

[Eric Seidel (no email)]

I believe this only applies to SVG objects where pointer-events is set to visiblePainted or painted.  In any case since visiblePainted is default, this is very common and should be fixed.  Attaching a simpler test case.

[Eric Seidel (no email)]

Created attachment 16403 [details]
simple test case (not ready for DRT)

Unfortunately due to bug 15289 the clip is not completely empty, mousing over 0, 0 should actually change the cursor... but the test is close enough. :)

[Hiren Joshi]

Created attachment 53543 [details]
Another test case for this bug

Another test case for this bug - still present in WebKit r57509 Mac OS X 10.6.

Behaviour is as expected in Firefox 3.6.3

[Dirk Schulze]

Created attachment 57681 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 57681 [details]
Patch

In general that looks fine, though I'd like to see the PE_FILL stuff encapsulated. How about passing style()->pointerEvents() and HitTestRequest ot PointerEventsHitRules? It could either grab the style()->pointerEvents() or set it to PE_FILL when request.svgClipContent() is true? Several other comments leading to r-:

WebCore/rendering/RenderPath.h:50
 +      bool fillContains(const FloatPoint&, bool requiresFill = true, WindRule = RULE_NONZERO) const;
Please use a parameter name here, looks odd.

WebCore/rendering/RenderSVGResourceClipper.cpp:289
 +          IntPoint hitPoint = IntPoint();
This is unncessary, no need to call the assignment operator.

WebCore/rendering/RenderSVGResourceClipper.h:58
 +      bool hitTestClipContent(const FloatRect&, FloatPoint);
Please pass FloatPoint as const-reference as well.

LayoutTests/svg/dynamic-updates/script-tests/SVGClipPath-influences-hitTesting.js:26
 +  var rectElementFG = createSVGElement("rect");
Why not name it foreground/background instead of using abbrevations?

[Dirk Schulze]

Created attachment 57961 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 57961 [details]
Patch

Looks great, r=me. One typo to fix before landing:

LayoutTests/svg/dynamic-updates/script-tests/SVGClipPath-influences-hitTesting.js:34
 +  // Two rects are drawn. One in the bckground and one in the foreground. The rect
Typo "background".

[Dirk Schulze]

Committed r60761: <http://trac.webkit.org/changeset/60761>

[Dirk Schulze]

Comment on attachment 57961 [details]
Patch

Clearing review flag.