Bug 174977 - SVG use element inside a shadow tree cannot reference an element in the same tree
Summary: SVG use element inside a shadow tree cannot reference an element in the same ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: Safari 10
Hardware: Unspecified Unspecified
: P2 Critical
Assignee: Ryosuke Niwa
URL: https://github.com/w3c/webcomponents/...
Keywords: InRadar
Depends on: 191898 191920
Blocks: 148695
  Show dependency treegraph
 
Reported: 2017-07-31 01:01 PDT by fabian.schwarzkopf
Modified: 2019-04-12 18:35 PDT (History)
9 users (show)

See Also:


Attachments
Reproduction of unresolved SVG use element in shadow DOM (1.06 KB, text/html)
2017-07-31 01:01 PDT, fabian.schwarzkopf
no flags Details
Extended repro with w3.org example (3.60 KB, text/html)
2017-08-02 00:51 PDT, fabian.schwarzkopf
no flags Details
Adds the support (47.98 KB, patch)
2018-11-23 20:02 PST, Ryosuke Niwa
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description fabian.schwarzkopf 2017-07-31 01:01:32 PDT
Created attachment 316759 [details]
Reproduction of unresolved SVG use element in shadow DOM

If an SVG is nested in a shadow root, use elements which reference a defs element via href="#id" are not resolved.

See attachment for a simple reproduce. Tested on macOS Sierra (10.12.5), Safari 10.1.1 and Safari Technology Preview Release 36.

Once the SVG is moved outside of the shadow root (select the SVG element in the developer tools, document.body.appendChild($0)), the use element is resolved correctly. 
If the SVG is nested in more than one shadow root, the problem persists on each level where the SVG is still in a shadow root, i.e. moving it up one level does not resolve the problem.

Defs elements that are used with url(#id) work as expected, e.g. referencing fills.
Comment 1 Radar WebKit Bug Importer 2017-08-01 16:38:12 PDT
<rdar://problem/33665636>
Comment 2 Ryosuke Niwa 2017-08-01 20:00:34 PDT
It's not clear if this is even a bug since we don't really have a spec defining this behavior.
Comment 3 fabian.schwarzkopf 2017-08-02 00:51:54 PDT
Created attachment 316946 [details]
Extended repro with w3.org example
Comment 4 fabian.schwarzkopf 2017-08-02 00:56:11 PDT
(In reply to Ryosuke Niwa from comment #2)
> It's not clear if this is even a bug since we don't really have a spec
> defining this behavior.

I don't see why this is not part of the spec. Looking through https://www.w3.org/TR/SVG/struct.html#UseElement, a similar example can be found (first code snippet) which also does not work on Safari / Safari Tech Prev when inside a shadow root. I've added the same code snippet to the attached index.html repro and it does not work.
Comment 5 Ryosuke Niwa 2017-08-02 01:00:49 PDT
(In reply to fabian.schwarzkopf from comment #4)
> (In reply to Ryosuke Niwa from comment #2)
> > It's not clear if this is even a bug since we don't really have a spec
> > defining this behavior.
> 
> I don't see why this is not part of the spec. Looking through
> https://www.w3.org/TR/SVG/struct.html#UseElement, a similar example can be
> found (first code snippet) which also does not work on Safari / Safari Tech
> Prev when inside a shadow root. I've added the same code snippet to the
> attached index.html repro and it does not work.

Well, the problem is that we haven't figured out how referencing a fragment URL works inside a shadow tree yet. See https://github.com/w3c/webcomponents/issues/179
Comment 6 Sebastian Müller 2017-08-04 01:30:20 PDT
While I understand that it may be difficult to deal with references that cross shadow-dom boundaries, I don't see how a "self-contained" SVG should leave anything to be discussed or even specced: If the use element is in the very same DOM subtree as the defs section, the relative # url should resolve, just like it does resolve properly if the svg is used in a data url as a image source. Moving the self-contained (with no refernces to anything else outside the svg element) SVG image from the top-level DOM into a shadow dom should *not* break it completely. SVG defs belong to SVG just like SVG belongs to HTML5. If they don't work in shadow doms at all, then shadow dom does not support HTML5.

Not supporting SVGUseElements in shadow doms would be like not supporting querySelector("#someHash") inside shadow dom, or actually even worse, because this affects static image assets and not even javascript.

If you keep the implementation the way it is, this will be yet another reason for devs *not* to switch to shadow DOM. Because without being able to use vector graphics in shadow dom, devs will rather not use shadow dom than omit the graphics.
Comment 7 Ryosuke Niwa 2017-08-04 02:13:12 PDT
(In reply to Sebastian Müller from comment #6)
> While I understand that it may be difficult to deal with references that
> cross shadow-dom boundaries, I don't see how a "self-contained" SVG should
> leave anything to be discussed or even specced: If the use element is in the
> very same DOM subtree as the defs section, the relative # url should
> resolve, just like it does resolve properly if the svg is used in a data url
> as a image source.

We certainly need to define what that means. For example, what happens if there are multiple shadow trees in which there are two distinct #circle defined and referenced. From what you've described, it seems to indicate that each reference should be using the one defined in its own shadow tree. Unfortunately, this would not mesh well with the existing SVG 2.0 spec which states that use element's href uses a URL reference to reference the source element (see https://svgwg.org/svg2-draft/struct.html#UseElement) because fragment URLs do not have a mechanism to differentiate two IDs defined in each shadow tree. For my interpretation of what you're proposing to work, we need to invest a wholly different semantics for use element.

> Moving the self-contained (with no refernces to anything
> else outside the svg element) SVG image from the top-level DOM into a shadow
> dom should *not* break it completely.

That's a good first principle for making SVG work better with shadow trees. However, someone has to do the work of defining what it means to reference a SVG element inside a shadow tree in the same tree, from an inner shadow tree or from outside. Nothing is well defined in this regard.

> If you keep the implementation the way it is, this will be yet another
> reason for devs *not* to switch to shadow DOM. Because without being able to
> use vector graphics in shadow dom, devs will rather not use shadow dom than
> omit the graphics.

I've written https://perf.webkit.org/v3/ using shadow DOM and inline SVG everywhere (there is literally no image in that website) without use elements, and it has worked really well. I agree that SVG use element not working is a huge limitation of the shadow DOM API today but it would not totally undermine the usefulness of shadow DOM API in my opinion.

Anyway, I welcome your contribution to https://github.com/w3c/webcomponents/issues/179 so that we can define the behavior and implement it in WebKit.
Comment 8 Sebastian Müller 2017-08-04 03:02:08 PDT
(In reply to Ryosuke Niwa from comment #7)

> We certainly need to define what that means. For example, what happens if
> there are multiple shadow trees in which there are two distinct #circle
> defined and referenced. From what you've described, it seems to indicate
> that each reference should be using the one defined in its own shadow tree.

Yes - just like if you had two divs with the same id in two different shadow trees. querySelector('#theDuplicateId') works just as well in this case (inside the respective tree), or not?

> Unfortunately, this would not mesh well with the existing SVG 2.0 spec which
> states that use element's href uses a URL reference to reference the source
> element (see https://svgwg.org/svg2-draft/struct.html#UseElement) because
> fragment URLs do not have a mechanism to differentiate two IDs defined in
> each shadow tree. For my interpretation of what you're proposing to work, we
> need to invest a wholly different semantics for use element.

The by far most common use-case for the "use" element is to reference elements in a "defs" element in the *same* svg document in the web context. Referencing defs from another document is almost never used (there are many issues with that in web browsers). So even though it *can* be a full-fledged URL, in most cases it is simply the anchor and for this case, I think it is totally obvious what the behavior should be. The living SVG 2.0 spec (which is not implemented anywhere, yet) should probably be adjusted to work with the most frequent use-case. If it does not work at all with shadow dom, then no one will be using it. 

> 
> > Moving the self-contained (with no refernces to anything
> > else outside the svg element) SVG image from the top-level DOM into a shadow
> > dom should *not* break it completely.
> 
> That's a good first principle for making SVG work better with shadow trees.
> However, someone has to do the work of defining what it means to reference a
> SVG element inside a shadow tree in the same tree, from an inner shadow tree
> or from outside. Nothing is well defined in this regard.

I totally agree with you here - but I don't believe it is necessary to have a solution for all absurd corner-cases (referencing *across* shadow-dom boundaries!?) before you can get the main use-case (crossing no boundaries) to work. 

I also don't understand at all why  fill="url(#someId)" should work, whereas href="#someId" does not. Can you explain why one thing is simple, whereas the other one is not?


> I agree that SVG use element not
> working is a huge limitation of the shadow DOM API today but it would not
> totally undermine the usefulness of shadow DOM API in my opinion.

Until SVG use elements work in Safari in the context of shadow dom "as expected", we are going to have to tell our customers either not to use shadow DOM or not to use Safari. It's as simple as that. I know what their choice will be...

> 
> Anyway, I welcome your contribution to
> https://github.com/w3c/webcomponents/issues/179 so that we can define the
> behavior and implement it in WebKit.

Thanks!
Comment 9 Sebastian Müller 2017-08-08 05:40:39 PDT
(In reply to Ryosuke Niwa from comment #7)
> That's a good first principle for making SVG work better with shadow trees.
> However, someone has to do the work of defining what it means to reference a
> SVG element inside a shadow tree in the same tree, from an inner shadow tree
> or from outside. Nothing is well defined in this regard.

The guys that work on the spec agree with me that at least the case where the SVG is fully contained in its shadow tree should work "as expected" and they feel this is defined well enough in the spec:

See https://github.com/w3c/webcomponents/issues/179#issuecomment-320879635

"I think everyone would agree that this should work. What clarification is required in SVG spec?"

So if you still feel that the spec is not well enough defined for this case, please go ahead and tell them what statement in the spec would change your mind if added. 
Otherwise *they* won't be changing the spec and *you* won't be changing the implementation and Safari's SVG implementation *will stay mostly unusable* in shadow trees.
Comment 10 Akshay Jat 2018-05-25 03:02:13 PDT
What is progress of this issue?
I am facing the same problem
Comment 11 Sebastian Müller 2018-05-25 05:18:28 PDT
(In reply to Akshay Jat from comment #10)
> What is progress of this issue?
> I am facing the same problem

There is no progress on this issue, neither here, nor at the spec. The webkit guys are waiting for the spec guys and the spec guys don't care about SVG in shadow doms: https://github.com/w3c/webcomponents/issues/179

There is absolutely no one working or even willing to work on changing that. IMHO this is a clear sign of failure of webcomponents (there are other severe bugs, but this one is a spec bug). I will not be using or would be recommending it to anyone until this has changed. If you *must* use web components in Safari and want to use SVG, make sure to put the SVG top-level onto the page. It would take at least one more year to get this to work, even if the guys from the spec started working on it today, so don't count on this becoming fixed in the near future... :-(
Comment 12 Ryosuke Niwa 2018-08-01 19:50:38 PDT
I'm following up with the spec issue again. I think we need more involvement from SVG WG and relevant browser vendors.
Comment 13 Ryosuke Niwa 2018-11-12 19:00:05 PST
Now that we've reached a rough consensus at W3C TPAC, we can implement the agreed behavior in WebKit, which is basically what Gecko & Chrome already implemented.
Comment 14 Ryosuke Niwa 2018-11-14 14:15:06 PST
Hm... I thought we had a consensus but it's not all that clear anymore. Will follow up with standards folks more so that we can at least make SVG use element work in the shadow tree.

Thanks for the patience!
Comment 15 Ryosuke Niwa 2018-11-23 20:02:04 PST
Created attachment 355552 [details]
Adds the support
Comment 16 Ryosuke Niwa 2018-11-23 20:02:40 PST
Don't be alarmed by the raw size of the patch. Most of that is from the tests!
Comment 17 Ryosuke Niwa 2018-11-26 14:48:10 PST
Committed r238524: <https://trac.webkit.org/changeset/238524>
Comment 18 Ryosuke Niwa 2019-04-12 18:35:04 PDT
This fix shipped in iOS 12.2 and Safari 12.1 on macOS.