Bug 15162 - hit testing does not respect clip paths
Summary: hit testing does not respect clip paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dirk Schulze
URL:
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2007-09-08 09:07 PDT by jay
Modified: 2011-01-02 03:24 PST (History)
2 users (show)

See Also:

Attachments
clip-path with anchor demo (910 bytes, image/svg+xml)
2007-09-08 09:08 PDT, jay 		no flags Details
simple test case (not ready for DRT) (514 bytes, image/svg+xml)
2007-09-26 14:17 PDT, Eric Seidel (no email) 		no flags Details
Another test case for this bug (1.31 KB, image/svg+xml)
2010-04-16 11:23 PDT, Hiren Joshi 		no flags Details
Patch (24.76 KB, patch)
2010-06-02 12:46 PDT, Dirk Schulze 		no flags Details | Formatted Diff | Diff
Patch (26.95 KB, patch)
2010-06-04 22:17 PDT, Dirk Schulze 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description jay 2007-09-08 09:07:23 PDT 
in the attachment, only the visible area should be active, not the whole anchor.

also the cursor remains an arrow, should change to hand.
Comment 1 jay 2007-09-08 09:08:28 PDT 
Created attachment 16228 [details]
clip-path with anchor demo
Comment 2 Eric Seidel (no email) 2007-09-24 16:32:15 PDT 
Both are hit testing issues (one is not seeming to respect clip-paths during hit testing) but probably should be separate bugs.
Comment 3 Eric Seidel (no email) 2007-09-26 14:14:17 PDT 
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.
Comment 4 Eric Seidel (no email) 2007-09-26 14:17:00 PDT 
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. :)
Comment 5 Hiren Joshi 2010-04-16 11:23:34 PDT 
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
Comment 6 Dirk Schulze 2010-06-02 12:46:44 PDT 
Created attachment 57681 [details]
Patch
Comment 7 Nikolas Zimmermann 2010-06-04 01:41:08 PDT 
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?
Comment 8 Dirk Schulze 2010-06-04 22:17:26 PDT 
Created attachment 57961 [details]
Patch
Comment 9 Nikolas Zimmermann 2010-06-06 03:59:41 PDT 
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".
Comment 10 Dirk Schulze 2010-06-06 05:27:19 PDT 
Committed r60761: <http://trac.webkit.org/changeset/60761>
Comment 11 Dirk Schulze 2011-01-02 03:24:17 PST 
Comment on attachment 57961 [details]
Patch

Clearing review flag.