Clean up error handling in declarative shadow DOM code#47558
Merged
chromium-wpt-export-bot merged 1 commit intomasterfrom Aug 10, 2024
Merged
Clean up error handling in declarative shadow DOM code#47558chromium-wpt-export-bot merged 1 commit intomasterfrom
chromium-wpt-export-bot merged 1 commit intomasterfrom
Conversation
This CL started as an implementation of exception throwing from the parsing code for DSD. However, as discussed in the issue: whatwg/html#10527 the desire is not to add such exceptions, mostly for fear of opening new XSS endpoints. So that bit has been ripped out of the CL, and what's left is a bit of a cleanup that at least moves all errors (including wrong-mode-string) to a central place in `ErrorMessageForAttachShadow`. This also allowed me to remove the `DeclarativeShadowRootMode` enum and some related code. I also added a few small explicit tests that exceptions are not thrown/fired during HTML parsing. Since the spec says exceptions should be fired in this case, I will put up a spec PR soon. For now, though, this is not a behavioral change for Chrome, so no need to do an I2S or anything. The spec, currently: -https://html.spec.whatwg.org/#parsing-main-inhead Start at `A start tag whose tag name is "template"`, then Otherwise, and then point 8, which is: > Attach a shadow root with declarative shadow host element, mode, > clonable, serializable, delegatesFocus, and "named". If an exception > is thrown, then catch it, report it for declarative shadow host > element's relevant global object, insert an element at the adjusted > insertion location with template, and return. Change-Id: I0a8e2c8fa1301efdd28f9bb786f9ca8d3596c2fd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5750587 Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1339961}
adae3bf to
5730a6d
Compare
wpt-pr-bot
approved these changes
Aug 10, 2024
Collaborator
wpt-pr-bot
left a comment
There was a problem hiding this comment.
The review process for this patch is being conducted in the Chromium project.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This CL started as an implementation of exception throwing from
the parsing code for DSD. However, as discussed in the issue:
whatwg/html#10527
the desire is not to add such exceptions, mostly for fear of
opening new XSS endpoints. So that bit has been ripped out of
the CL, and what's left is a bit of a cleanup that at least
moves all errors (including wrong-mode-string) to a central
place in
ErrorMessageForAttachShadow. This also allowed meto remove the
DeclarativeShadowRootModeenum and somerelated code.
I also added a few small explicit tests that exceptions are
not thrown/fired during HTML parsing.
Since the spec says exceptions should be fired in this case,
I will put up a spec PR soon. For now, though, this is not
a behavioral change for Chrome, so no need to do an I2S or
anything.
The spec, currently:
-https://html.spec.whatwg.org/#parsing-main-inhead
Start at
A start tag whose tag name is "template", then Otherwise,and then point 8, which is:
Change-Id: I0a8e2c8fa1301efdd28f9bb786f9ca8d3596c2fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5750587
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1339961}