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

Status: RESOLVED DUPLICATE of bug 376027

(Core :: SVG, defect)

Description

[Matt Sidor]

Attachment: 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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: 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.

Comment 2

[Daniel Holbert [:dholbert]]

(I'm reproducing using current nightly on linux)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: 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).

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: 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.

Comment 5

[Daniel Holbert [:dholbert]]

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).)

Comment 6

[Daniel Holbert [:dholbert]]

(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.

Comment 7

[Daniel Holbert [:dholbert]]

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.)

Comment 8

[Robert Longson [:longsonr]]

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.

Comment 9

[Daniel Holbert [:dholbert]]

Yeah, I agree. Duping.

Comment 10

[Matt Sidor]

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!!!!!

Comment 11

[Matt Sidor]

Actually, I spoke too soon, sorry. It's still happening in Firefox because of bug 376027. Argh.

Comment 12

[Daniel Holbert [:dholbert]]

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)...

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: 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.

Comment 14

[Matt Sidor]

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.

Comment 15

[Daniel Holbert [:dholbert]]

No worries. Good luck! :)

Comment 16

[Matt Sidor]

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

Thanks again!

Comment 17

[Daniel Holbert [:dholbert]]

Hooray, glad to hear it! Thanks for filing bugs.