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

[Shadow]: Element.createShadowRoot() throws an exception for some elements (bugzilla: 27844) #102

Closed
hayatoito opened this issue May 25, 2015 · 2 comments
Assignees

Comments

@hayatoito
Copy link
Contributor

Title: [Shadow]: Element.createShadowRoot() throws an exception for some elements (bugzilla: 27844)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844


comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c0
Philip Jägenstedt wrote on 2015-01-16 12:38:29 +0000.

Per spec createShadowRoot() is always supported, but in Blink it will throw an exception for HTMLCanvasElement, HTMLFieldSetElement, HTMLImageElement, HTMLKeygenElement, HTMLMediaElement, HTMLMeterElement, HTMLProgressElement, HTMLSelectElement, HTMLTextAreaElement, HTMLFrameElement and HTMLIFrameElement.

Make this part of the spec so that the circumstances in which createShadowRoot() works is documented and consistent between implementations?


comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c1
Olli Pettay wrote on 2015-01-16 12:43:14 +0000.

Or rather fix the implementation? Why would we need to have the
special cases for those seemingly random elements?

Or, should it be defined on which elements createShadowRoot() works.
Like limit it to

and (and some svg element) if implementations have issues with other elements?

I'd prefer not having special cases, so making it work everywhere.


comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c2
Philip Jägenstedt wrote on 2015-01-16 13:17:55 +0000.

I'm not sure how much work it would be to make it work everywhere, it was tried for but abandoned once:
http://trac.webkit.org/changeset/122824
http://trac.webkit.org/changeset/140097

Having it just work everyone would be nice, but if that isn't going to be prioritized it seems preferable to at least document the interoperable subset, if not making it normative.

Dimitri and Hayato are in a better position to say what's likely to happen in Blink, I just noticed this and wanted to raise it.


comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c3
Dimitri Glazkov wrote on 2015-01-16 16:59:31 +0000.

(In reply to Philip Jägenstedt from comment #0)

Per spec createShadowRoot() is always supported, but in Blink it will throw
an exception for HTMLCanvasElement, HTMLFieldSetElement, HTMLImageElement,
HTMLKeygenElement, HTMLMediaElement, HTMLMeterElement, HTMLProgressElement,
HTMLSelectElement, HTMLTextAreaElement, HTMLFrameElement and
HTMLIFrameElement.

Make this part of the spec so that the circumstances in which
createShadowRoot() works is documented and consistent between
implementations?

I always viewed these as our implementation limitations (http://crbug.com/234020), but I am open to the idea that the spec needs to change as well.


comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c4
Philip Jägenstedt wrote on 2015-01-16 22:41:28 +0000.

Yeah, they clearly are, but is it a lot of work to fix? I don't have a strong opinion really, but the next browser to implement and ship Shadow DOM should at least be made aware of the problem so that they can decide what to do. Maybe they'll even have similar limitations.


comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c5
Hayato Ito wrote on 2015-01-19 02:27:46 +0000.

This should be considered as implementation issues, basically.

We had spent some time on supporting some of these elements in WebKit/Blink, but things didn't went smoothly. The efforts were suspended by other higher-priority tasks.

There is no reason for other UAs not to support these elements.

Let me close this issue since we don't have to update the spec so that it throws Exceptions for these elements. If you have any idea how to update the specs other than that, please feel free to reopen this.


comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c6
Philip Jägenstedt wrote on 2015-01-19 08:31:24 +0000.

CC William Chen who seems to have done most of the implementation in Gecko. If Mozilla people are OK with supporting the current situation then I have nothing to add.


comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c7
Hayato Ito wrote on 2015-05-20 09:33:40 +0000.

Let me reopen this because we are dropping multiple shadow roots.


comment: 8
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c8
Anne wrote on 2015-05-21 01:18:12 +0000.

It seems the easiest would be here to restrict this method (by throwing a "NotSupportedError" for the elements that have a 'binding' property applied to them per HTML's Rendering section. Probably by enumerating the specific list of elements this needs to happen for within the definition of createShadowRoot().


comment: 9
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c9
Hayato Ito wrote on 2015-05-21 03:51:54 +0000.

(In reply to Anne from comment #8)

It seems the easiest would be here to restrict this method (by throwing a
"NotSupportedError" for the elements that have a 'binding' property applied
to them per HTML's Rendering section. Probably by enumerating the specific
list of elements this needs to happen for within the definition of
createShadowRoot().

'binding' property is https://html.spec.whatwg.org/#bindings, right?

I'm a bit confused. Why do we need the list of elements in the shadow dom spec if 'binding' is enough to decide such an element which should throw NotSupportedError? Is that a tentative plan until we can upstream it to each element's definition?


comment: 10
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c10
Anne wrote on 2015-05-21 04:12:57 +0000.

The main problem is that HTML's Rendering section is not required (it's a should at best) whereas this needs to be requirement (a must). But we can link to HTML to indicate how the list was formed.

@hayatoito hayatoito self-assigned this May 29, 2015
@hayatoito
Copy link
Contributor Author

@annevk
I'm afraid that I don't understand fully what https://www.w3.org/Bugs/Public/show_bug.cgi?id=27844#c10 means, however, let me update the spec so that it throws an Exception.
I'll ping you so that it can be reviewed by you.

I'm also thinking that createShadowRoot() should throw a similar exception if it is called more than once with the same context object. In the current spec, it is defined so it returns null.

https://w3c.github.io/webcomponents/spec/shadow/#widl-Element-createShadowRoot-ShadowRoot-ShadowRootInit-shadowRootInitDict

I'm not sure which is better, throwing an Exception or returning null. I am thinking throwing an Exception is better also in this case.

@hayatoito hayatoito added the v1 label Jun 1, 2015
@hayatoito
Copy link
Contributor Author

@annevk
I've updated the spec so that it throws an exception. If you have an idea of an improvement, please let me know it.

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

2 participants