-
Notifications
You must be signed in to change notification settings - Fork 129
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
Add link to declarative shadow dom opt-in #300
base: main
Are you sure you want to change the base?
Conversation
As I mentioned there, I don't think we should add this feature to this API. XMLHttpRequest is essentially frozen and new functionality should go into |
I agree, and I didn’t miss that feedback. This change explicitly sets the “include shadow roots” flag to “deny”, so that XHR cannot be used to parse DSD content. Without this change to XHR, it will allow DSD parsing. So effectively, this is “no change”. |
I missed that, is there a reason "deny" is not the default, given that it's "unsafe"? |
I'm open to suggestions on how I implemented this in the HTML spec (and in code), but as it stands, the "include shadow roots" flag is tri-state. It can be unset, or explicitly "allow" or "deny". The reason is that for fragment parsing, unset means "don't allow" DSD content. But for non-fragment parsing, unset means "allow" DSD content. The explicit cases allow that default behavior to be overridden, e.g. here for XHR, where the XHR document is parsed with a non-fragment parser, but we still don't want to allow DSD content. Or the opposite case for |
Interesting, I guess I need to look at the specific PRs, but it seems that it could be a boolean everywhere that defaults to false, and when we invoke the main parser for documents we set it to true. |
Ok. I'll be interested to hear your suggestions for implementing it that way. I found in the implementation that there are multiple places we invoke the parser, so this was the least overhead approach to avoid touching most of them. But always open to improvements here! |
This is obsolete now, right @mfreed7? |
No, I think we still need this, to ensure XHR doesn't parse DSD content. It does need an update, since "include shadow roots state" has been renamed. |
But isn't the default of this state deny? Oh, I guess it also has unset, but I still don't really understand that third state. |
The default is "unset" which is interpreted as "allow" for document parsing and "deny" for synchronous parsing. |
Can we make it deny and have page loading overwrite it? Usually when we have tri-states there are actually three different behaviors. |
This PR goes along with the two in HTML and DOM to add declarative Shadow DOM to the spec. This PR goes along with the discussion at #912, to opt-in to the fragment parser for declarative Shadow DOM.
Preview | Diff