-
Notifications
You must be signed in to change notification settings - Fork 73
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
Update IDL for script enforcement #484
Conversation
4e7c522
to
3661183
Compare
cc @mbrodesser-Igalia might be of interest to you as you start implementing more in Gecko |
3661183
to
9944d22
Compare
43720d4
to
99c8fb7
Compare
1da7777
to
8c008d2
Compare
8c008d2
to
5e266cf
Compare
@annevk hopefully this is now in a better shape. Shadows the properties onto HTMLScriptElement rather than messing with Node/textContent and Element/innerText. Uses the IDL attribute where possible so algorithms are mostly just setting the internal slot and calling the parent steps HTMLScriptElement/textContent uses a union type instead of the IDL extended attribute to avoid issues with nullable IDL type handling. |
2d2077c
to
dedb72a
Compare
Updated this to link to the correct steps in the DOM and html specs so it should read better. |
- Node/textContent, and Element/innerText are both now shadowed on HTMLScriptElement. HTMLScriptElement/textContent uses a union type rather than [StringContext] because it uses a nullable type.
ab520f1
to
18a76ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have the same question as I have for the HTML PR. Do we need to store a redundant copy of the text, or is the actual model invalidation of some kind of trusted bit.
Currently: We validate during prepare the text so we do need to store the text. The "invalidation" is currently only something we propose for the parsing behaviour. It's possible that if all the ways to change the children of a script element end up firing children changed steps, and we get the neccessary changes there (still need to open a DOM PR) that we can do away with the script text entirely and just use invalidation and a trusted boolean "slot". I'll raise a separate issue to discuss that as that's tangental to this PR itself. |
Raised #507 to discuss the "slot" vs invalidation model. Aside from that do the changes to the IDL here make sense? |
I think so. |
I'm going to go ahead and get this merged, we'll get another chance when I upstream it to HTML to double check it all makes sense within the wider context of the full script enforcement. |
SHA: 5592e30 Reason: push, by lukewarlow Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
HTMLScriptElement/textContent uses a union type rather than [StringContext] because it uses a nullable type.All of the properties are using union types as that seems to be the direction the spec is going to go in. But I can revert that commit and go back to using the extended attribute.
This PR is part of 1 of addressing #437 it specifically doesn't address #483 or #252
Preview | Diff