RESOLVED FIXED 18340
Elements with display="none" in a <clipPath> still contribute to clipping path
https://bugs.webkit.org/show_bug.cgi?id=18340
Summary Elements with display="none" in a <clipPath> still contribute to clipping path
Cameron McCormack (:heycam)
Reported 2008-04-07 04:21:56 PDT
Elements inside a <clipPath> that have display="none" should not contribute to the clipping path, but they currently do. I thought perhaps that simply replacing SVGStyledTransformableElement::toClipPath() with: virtual Path toClipPath() const { return renderer() ? toPathData() : Path(); } would work (on the assumption that there'd be no renderer for elements with display="none"), but in fact there is no renderer for any element in the <clipPath>.
Attachments
First attempt (26.95 KB, patch)
2008-04-09 12:25 PDT, Rob Buis
eric: review-
Now with improved testcase (26.97 KB, patch)
2008-04-10 11:12 PDT, Rob Buis
eric: review+
Cameron McCormack (:heycam)
Comment 1 2008-04-07 04:27:43 PDT
Relatedly, when computing the clipping path from a given <clipPath>, each child element of it needs to have its clipping path (i.e., what's specified by the 'clip-path' property) applied before being unioned into the resulting clipping path.
Rob Buis
Comment 2 2008-04-09 12:25:14 PDT
Created attachment 20440 [details] First attempt This should do it. I am not sure whether I did the test stuff right, as in that the png is in the right dir... The second remark from Cameron handles I think a different bug, and IMHO we need a new bug and testcase to proof that we have this problem. Cheers, Rob.
Cameron McCormack (:heycam)
Comment 3 2008-04-09 22:26:07 PDT
Filed bug 18398 with test case for the related bug.
Eric Seidel (no email)
Comment 4 2008-04-09 23:56:03 PDT
Comment on attachment 20440 [details] First attempt Ok, so two things: 1. style error: if ( pathStyle->display() != NONE ) (no spaces inside the ( )) 2. You should test visibility: hidden as well. What's the expected behavior there? Also: // FIXME: How do we know the element has done a layout? We could pretty easily test the not-yet-layout case, but I'm not sure that layout even matters here, since we're calling a DOM method toClipPath(), however I'm not sure the DOM can really get the right proportions... how does it resolve lengths correctly? We at least need a visibility: hidden test, IMO.
Cameron McCormack (:heycam)
Comment 5 2008-04-10 00:06:16 PDT
(In reply to comment #4) > 2. You should test visibility: hidden as well. What's the expected behavior > there? visibility='hidden' (but display!='inline') elements still contribute to the clip path.
Rob Buis
Comment 6 2008-04-10 11:12:49 PDT
Created attachment 20456 [details] Now with improved testcase Fix the style issue and make the testcase check for visibility=hidden too. Cheers, Rob.
Eric Seidel (no email)
Comment 7 2008-04-10 23:27:18 PDT
Comment on attachment 20456 [details] Now with improved testcase If you update the testcase to mention the purpose of visibility="hidden" (that we're testing to make sure that it does not affect clipping) then r=me. I don't need to see the patch again before you land.
Rob Buis
Comment 8 2008-04-11 00:10:29 PDT
Landed in r31805.
Note You need to log in before you can comment on or make changes to this bug.