svg not honoring "clip-path" in a display:none subtree

RESOLVED DUPLICATE of bug 376027

Status

()

Core
SVG
RESOLVED DUPLICATE of bug 376027
3 years ago
3 years ago

People

(Reporter: Matt Sidor, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
Created attachment 8556787 [details]
Water-Energy-Graphs.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36

Steps to reproduce:

1) SVGs for little bar graphs are dynamically written inline by an Angular.js process

2) Half the SVGs are shown on page load (energy bar graphs), half the SVGs are hidden ("display: none") on page load (water bar graphs)

3) When you click on the water drop icon in the upper-right corner of the page, the energy graphs are hidden ("display: none") and the water graphs are shown ("display: block" or "display: inline").


Actual results:

The green water bar graphs look different from the green energy graphs, even though they are both using identical SVG code. (For the water graphs, the green hash is on top of the orange border instead of the orange border being on top of the green hash.) The only difference seems to be that the water graphs are initially hidden.


Expected results:

Both sets of green graphs should look the same, with the orange border on top of the green hash instead of the green hash on top of the orange border.

This problem does not occur in Chrome or Safari.
Created attachment 8556856 [details]
testcase 1 (semi-reduced)

I can reproduce this.

The green hashmarks are supposed to be clipped using "clip-path", but the clipping is failing for some reason. It works if I find the corresponding electricity <svg> in the DOM inspector and delete it from the page, though.

So, I think there's an ID clash between the two SVG subtrees -- for electricity and for water.  When you hide the electricity one & show the water one, the water one grabs its clip-paths based on ID, and the first elements that match those IDs are in a display:none subtree (the corresponding <svg> tree for the electricity graph). And that makes us fail to clip.
(I'm reproducing using current nightly on linux)
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Version: 36 Branch → Trunk
Created attachment 8556865 [details]
testcase 2: two clip paths with same ID, neither one is display:none

Here's a testcase with two clip-paths that have the same ID, to show how browsers handle that.

Gecko (Firefox) picks the first one in the DOM (which I believe is correct) -- showing lots of purple but no red.

Blink (Chrome) & WebKit (Midori) pick the second one, showing lots of red (because the second clip-path happens to be smaller).
Created attachment 8556868 [details]
testcase 3: test for display:none clip-path

This testcase demonstrates that all browsers agree that a display:none clip-path gets nerfed. (So, the purple rect ends up not being clipped at all.)  This displays the same in all browsers.
Actually, testing in IE 11:
 - IE 11 agrees with Firefox in testcase 2 (it picks the first element whose ID matches, and doesn't show any red)
 - IE 11 *disagrees* with everyone else (Gecko, WebKit, & Blink) on testcase 3 -- it renders testcase 3 as fully-lime.  This means it treats the missing clip-rect target as meaning to *not render the clipped thing at all*.  That's interesting. It also means IE breaks on the original page, in the "water" view, if you adjust the zoom-level -- they show the green stripes at first when you switch into "water" mode, but the green stripes disappear when you adjust the zoom level. This is consistent with IE's rendering of testcase 3 (and probably an IE bug).

ANYWAY. I'm tempted to think that Firefox's behavior here is correct -- in particular, that:
   (a) we should choose the first element whose ID matches the specified "clip-path", to use for clipping.
   (b) if the element we're using for our clip-path is display:none, we shouldn't clip.

(IE agrees with us on (a), as shown by testcase 2, and Blink/WebKit agree with us on (b).)
(In reply to Daniel Holbert [:dholbert] from comment #5)
>    (b) if the element we're using for our clip-path is display:none, we
> shouldn't clip.

Hmm, actually, it looks like the SVG spec says everyone is wrong on this; "display:none" isn't supposed to impact clipPath usability. The spec says:
 # ‘clipPath’ elements are available for referencing
 # even when the ‘display’ property on the ‘clipPath’
 # element or any of its ancestors is set to none.

...just above this anchor in the spec:
 http://www.w3.org/TR/SVG11/masking.html#ClipPathProperty

So I think we are technically wrong here, and my assumption (b) from comment 5 is incorrect (which makes our rendering wrong). Digging a bit, it looks like this issue is covered by bug 376027.
Depends on: 376027
Summary: Inline SVGs not rendering consistently → svg not honoring "clip-path" in a display:none subtree
FWIW, I filed bugs for WebKit & Blink, on them picking the wrong clipPath element (and getting "testcase 2" wrong), which is how they accidentally end up with the desired rendering on the original URL here:
  https://bugs.webkit.org/show_bug.cgi?id=141068
  https://code.google.com/p/chromium/issues/detail?id=453722

Matt, just so you know (in case you're involved with this website on ucdavis.edu): the website is effectively *depending* on this WebKit/Blink bug to get the rendering right.  Really, it should be using unique "id" attributes for each of the clipPath elements, so that it's unambiguous which element a "clip-path" attribute is pointing to.  That would fix the issue that you're hitting in Firefox.

(There is also a Firefox bug here -- we should support using a "display:none" clip-path -- but the site here doesn't really want to ask for a "display:none" clipPath. It's only doing so accidentally, due to this ID re-use.)
Picking the first ID is correct per the relevant specifications and not handling a display:none clipPath is covered by bug 376027. We should probably simply close this as a duplicate of bug 376027.
Yeah, I agree. Duping.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
No longer depends on: 376027
Resolution: --- → DUPLICATE
Duplicate of bug: 376027
(Reporter)

Comment 10

3 years ago
Daniel, thank you so much for all of your detective work on this last night. I'm really grateful for all the time and energy you invested to figure this out. Thanks also for submitting bug reports to WebKit and Blink.

I assigned a unique ID to each <clipPath> and the problem has now been fixed. I'll be uploading the changes to the production server later today.

THANK YOU!!!!!
(Reporter)

Comment 11

3 years ago
Actually, I spoke too soon, sorry. It's still happening in Firefox because of bug 376027. Argh.
You're welcome! Though... [comment 11] D'oh.

If you've got unique IDs on each of the clipPath elements, then I don't think bug 376027 should be able to bite you (because your visible SVG subtrees should all contain their own clipPath elements, which should be visible & should now be actually addressable via their unique IDs)...
Created attachment 8557177 [details]
testcase 4: clipPath element dynamically going from display:none to display:inline (after 1 second)

This testcase shows that we do correctly render a clipPath once it's given a non-"none" display value.
(Reporter)

Comment 14

3 years ago
Sorry Daniel, I think I am jumping too fast on this without fully testing it. I think it may be a problem with my code for 'unique' IDs. Give me a couple minutes to hash this out and I'll get back to you. My bad for commenting too fast on this.
No worries. Good luck! :)
Attachment #8557177 - Attachment description: testcase 4: clipPath element dynamically going from display:none to display:inline → testcase 4: clipPath element dynamically going from display:none to display:inline (after 1 second)
(Reporter)

Comment 16

3 years ago
Alrighty, it was my code after all. The IDs weren't actually unique. Now that they are, the problem is fixed.

Thanks again!
Hooray, glad to hear it! Thanks for filing bugs.
You need to log in before you can comment on or make changes to this bug.