Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the use element shadow tree to closed #363

Open
boggydigital opened this issue Nov 9, 2017 · 7 comments
Open

Change the use element shadow tree to closed #363

boggydigital opened this issue Nov 9, 2017 · 7 comments

Comments

@boggydigital
Copy link
Contributor

https://svgwg.org/svg2-draft/single-page.html#struct-UseShadowTree

"The shadow tree is open (inspectable by script), but read-only. Any attempt to directly modify the elements, attributes, and other nodes in the shadow tree must throw a NoModificationAllowedError."

needs to be changed to "the shadow tree is closed" and some paragraphs around need to be modified to reflect that.

As discussed in the room: no implementation does this today and we won't be able to provide tests that would have >=2 implementations.

@fsoder
Copy link

fsoder commented Nov 9, 2017

...no implementation does this today...

Blink does. That's still < 2 of course.

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Nov 9, 2017

@fsoder, quick test in Chrome 62 shows the shadowRoot property on a <use> element is reporting null, so I don't think it's currently implemented as an open tree. https://codepen.io/AmeliaBR/pen/6b3539813c1884acc3a2ee9be036e80b?editors=1111

As I recall, the decision to go with "open" was to make it possible to create backwards compatible equivalents to the SVG 1.1 behavior. But the SVG 1.1 spec was never implemented interoperably, and current implementations have moved even farther away from it, so there is unlikely to be a interop issue with making it closed.

If the shadow tree is closed, then you can drop the SVGElementInstance partial interface and the instanceRoot/animatedInstanceRoot properties on SVGUseElement, which were both about preserving backwards compatibility with SVG 1.1 inspectable shadow trees. You also probably don't need the separate SVGUseElementShadowRoot interface.

There are probably other places where the prose can be simplified, or where implementation details (e.g., re stylesheets and animations) can be made more flexible (so long as the final rendering is the same). If you're volunteering to make the edits @boggydigital, feel free to check in with me anything where you're not sure why I wrote it the way I did.

@fsoder
Copy link

fsoder commented Nov 9, 2017

I meant that it was implemented as a "closed" tree. I thought that was the behavior for which tests was going to be provided, but apparently I misinterpreted.

(FWIW, I don't think using an "open" tree ever had a chance of even coming close to what was in SVG 1.1 - SVGElementInstances were very special "entities", not being elements et.c. But digressions...)

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Nov 10, 2017

Has enough of this behavior been implemented post-the Blink/WebKit split that they would count as separate implementations? If so, that's your two.

I don't think Firefox and MS Edge are yet fully shifted over to a shadow DOM model, although they would probably pass some tests, re event retargetting and so on.

Re: digressions. Feel free to peruse the current spec test for how I was suggesting a modern shadow DOM element tree could pretend to also be a SVG 1.1 element-instance tree. (They're elements, but everything is read-only! And immutable! and they have extra properties!) As I commented above, it will streamline the spec considerably to drop it.

@fsoder
Copy link

fsoder commented Nov 10, 2017

Implementing as a closed shadow tree is a post-fork thing. I don't know how the WebKit implementation looks like these days (lots of water under the bridge on the Blink side definitely.)

I think it suffice to say that read-only, immutable Element would be a pretty new concept. (Read-only properties - i.e animVals - are challenging enough I think...)

@boggydigital
Copy link
Contributor Author

Not blocking updated 2.0 CR publication - assigning 2.0 Recommendation milestone to clean this up before 2.0 REC

@AmeliaBR
Copy link
Contributor

Recording the resolution from 21 May 2018:

RESOLUTION: use closed shadow DOM and remove correspondingElement and correspondingUseElement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants