1366722 - SVG use element is invisible when certain transform is applied to SVG element

Status: RESOLVED DUPLICATE of bug 265894

(Core :: SVG, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

Attachment: testcase

See the attached testcase.

The testcase contains three arrows, the first and second uses <use> element to refer to a local <symbol> element, and the third is a direct copy of the content in the <symbol> element.

The second and third have "transform: scaleY(-1)" applied.

In Chrome and Edge, all three arrows are visible, while in Firefox, the second one is not visible.

This is reproducible on Windows, macOS, and Android, as far as I can tell.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

This seems to be a regression in this range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=762e95608da3&tochange=e794cef56df6

I would suspect bug 512525 and bug 734082 since they mentioned SVG :)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (with redundant -moz-transform styles added as well)

I can reproduce on Linux Nightly as well.

(Side note: to make the testcase work properly in Nightlies from around the regression range, you need to add prefixed "-moz-transform" declarations alongside the unprefixed ones.  This regression slightly predates our -moz-transform unprefixing in bug 745523. Here's a testcase with that tweak, for reference & for testing of old builds.)

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> This seems to be a regression in this range:
> https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=762e95608da3&tochange=e794cef56df6
> 
> I would suspect [...] bug 734082 since they mentioned SVG :)

Confirmed regression from bug 734082 locally, with targeted builds:
  https://hg.mozilla.org/mozilla-central/rev/05a339620439 is bad
  https://hg.mozilla.org/mozilla-central/rev/d3b11e443f04 is good  (parent of ^^)

(To my slight surprise, I can still build these 5-year-old source trees just fine on Ubuntu 17.04, using gcc 4.7)

jwatt, any chance you can take a look?  (I'm a little surprised that bug 734082 had an immediate behavior impact, since I'd thought it was part of SVG-painting-with-display-lists, which was preffed off at that point in time.)

Comment 4

[Robert Longson [:longsonr]]

You can see the second one if you set overflow: visible so the question is, is it supposed to be outside the SVG viewport?

If so it's expected to be invisible unless you specify overflow: visible. SVG 2 would like to change this but the performance implications of that are substantial so I don't think we can/should do that until or unless we can address that.

If not we're presumably positioning incorrectly?

If you modify the testcase to have overflow: visible; does it move position at all when bug 734082 lands?

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

It seems to me this is that we are positioning incorrectly.

If you change the transform to "transform: rotate(30deg)", it would be even more fun.

So it seems like we apply the transformation to both the SVG itself and the <use> element inside, which sounds insane to me.

Comment 6

[Robert Longson [:longsonr]]

No, that's entirely correct as the use element becomes an svg element.

https://www.w3.org/TR/SVG/struct.html#UseElement

The referenced ‘symbol’ and its contents are deep-cloned into the generated tree, with the exception that the ‘symbol’ is replaced by an ‘svg’.

I think therefore that this bug is invalid and the bug is in the other browsers and our previous implementation.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

That's wrong. The spec explicitly says the following:
> CSS2 selectors can be applied to the original (i.e., referenced) elements because they are part of the formal document structure. CSS2 selectors cannot be applied to the (conceptually) cloned DOM tree because its contents are not part of the formal document structure.
which means having ".b svg" applied on the cloned tree is wrong behavior. It doesn't match how CSS is expected to work in general.

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

But this makes it sound like a style system issue rather than an SVG issue, though. I'm not sure.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Anyway, it is still a regression from bug 734082 so restoring ni? jwatt.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 3: with "overflow: visible" on the missing SVG

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Robert Longson from comment #4)
> If you modify the testcase to have overflow: visible; does it move position
> at all when bug 734082 lands?

That's hard to say.

Superficially, it still *disappears* when bug 734082 lands, regardless of "overflow:visible" -- and then in current Nightlies, it's visible (at the lower position).

So bug 734082 made it disappear entirely, and then some later changeset made it show up at a lower position.  mozregression says that later changed in this range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=218785bab4bd&tochange=0fed3377c839
And in particular, I'm betting the relevant change there was:
> 6f025261b40d Jonathan Watt — Bug 378923 - Make non-root outer-<svg> respect |overflow:visible| (overflow="visible"). r=roc

(After that point, Nightlies match current Nightly on 'testcase 3' -- the two lower arrows are superimposed/crossed.)

Given the commit message on that ^ changeset, it seems that when the original behavior-change here happened (with bug 734082), we didn't respect "overflow:visible" on one of the <svg> elements involved, so it's hard to say where the out-of-bounds SVG content is painting until builds with bug 378923 landed.

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: testcase 4: using child selector instead of descendant selector (works correctly)

Per longsonr/xidorn discussion above, here's a testcase (based on "testcase 2") which uses child selector ">" rather than descendant selectors " ".  This produces behavior that matches other browsers, I think since it doesn't target the <use>-generated extra <svg> node.

Comment 13

[Daniel Holbert [:dholbert]]

So I think the only actual bug here (in current Nightly) is that CSS selectors are incorrectly being allowed to target the <svg> node in the cloned subtree.

That's covered by bug 265894 (whose expected behavior is clarified in bug 265894 comment 27), and also bug 1268431 (see dbaron's simpler testcase attached there).


So w.r.t. the "regression" -- I suspect bug 734082 really fixed a bug here -- previously, we were probably applying the transform to both <svg> elements but only actually rendering it for one of them -- and bug 734082 made us start rendering it for both of them (which results in the content being transformed out of view).  And of course we're not supposed to apply the transform to the <use>-generated SVG element in the first place, but that's bug 265894.