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: Macintosh 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

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.