1450250 - <svg:use> should use an actual shadow tree, instead of NAC.

Status: RESOLVED FIXED

(Core :: SVG, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

This is for soundness in general, since you can stash any kind of element in there and can cause non-desired stuff to run from layout, like in bug 1450181.

Also, it's needed to allow you to do <svg:use style="display: contents">, which works in other engines but not in Gecko.

Finally, it allows to simplify a lot of hacks that were added for that, like :-moz-use-shadow-tree-root, blocks_ancestor_combinators, etc...

Using NAC here is really a hack.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This will make a lot of stuff much nicer, I think we should do this.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1450250: Make svg:use use an actual shadow tree. r=heycam

This fixes a couple fuzz bugs and prevents special-casing <svg:use> even more in
bug 1431255.

Unfortunately not as many hacks went away as I'd have hoped, since we still need
to match document rules, see the linked SVGWG issues.

But blocks_ancestor_combinators goes away, which is nice since it's on a very
hot path.

Comment 3

[Bobby Holley (:bholley)]

Very happy to see this. Thanks Emilio!

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: SMIL document-related fixes.

Depends on D2154. This is a prerequisite to make some SMIL-in-use tests pass.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Make <animate> target the light DOM parent, not the flattened tree parent.

This matches chrome:

  https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/svg/animation/svg_smil_element.cc?l=260&rcl=325eafc631f396944801f17675a7092b216d60ca

And we rely on that to animate <use> with SMIL, like:

<use href="#foo">
  <animate ...>
</use>

There, the flattened tree parent of the <animate> element is null, because <use>
is a shadow root without slots.

Similarly we need to skip the GetCtx check which seems unneeded, and also is not
correct since <animate> can animate when it's out of the flattened tree.

Depends on D2192.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Preserve the behavior of GetOuterSVGElement so that it works for <animate>.

It's used by SVGAnimationElement, and otherwise it can't find the root element.
It also feels like the right thing to do, and didn't break any XBL+svg tests
(though we may as well get rid of those at this point).

Depends on D2192.

Comment 7

[Phabricator Automation]

Comment on attachment 8992341 [details]
Bug 1450250: Make svg:use use an actual shadow tree. r=heycam

Cameron McCormack (:heycam) has approved the revision.

https://phabricator.services.mozilla.com/D2154

Comment 8

[Phabricator Automation]

Comment on attachment 8992695 [details]
SMIL document-related fixes.

Cameron McCormack (:heycam) has approved the revision.

https://phabricator.services.mozilla.com/D2192

Comment 9

[Phabricator Automation]

Comment on attachment 8992707 [details]
Make <animate> target the light DOM parent, not the flattened tree parent.

Cameron McCormack (:heycam) has approved the revision.

https://phabricator.services.mozilla.com/D2194

Comment 10

[Phabricator Automation]

Comment on attachment 8992708 [details]
Preserve the behavior of GetOuterSVGElement so that it works for <animate>.

Cameron McCormack (:heycam) has approved the revision.

https://phabricator.services.mozilla.com/D2195

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d2a2444ad1
SMIL document-related fixes. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/91b097951606
Make <animate> target the light DOM parent, not the flattened tree parent. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/d335f602d578
Preserve the behavior of GetOuterSVGElement so that it works for <animate>. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/453d0a08f00c
Make svg:use use an actual shadow tree. r=heycam

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12092 for changes under testing/web-platform/tests

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d4d2a2444ad1
https://hg.mozilla.org/mozilla-central/rev/91b097951606
https://hg.mozilla.org/mozilla-central/rev/d335f602d578
https://hg.mozilla.org/mozilla-central/rev/453d0a08f00c

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged