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

Remove getHTML() shouldn't branch on shadow root's mode #45624

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 9, 2024

As with clonable and delegatesFocus, serializable is an explicit API and therefore fine from an encapsulation point of view.

For whatwg/html#10260.

As with clonable and delegatesFocus, serializable is an explicit API and therefore fine from an encapsulation point of view.

For whatwg/html#10260.
Copy link
Contributor

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this LGTM. I'll check out how it behaves on Chromium once this gets imported.

shadow-dom/declarative/gethtml.html Show resolved Hide resolved
@annevk annevk enabled auto-merge (squash) April 10, 2024 06:24
@annevk annevk merged commit dd2e51d into master Apr 10, 2024
17 checks passed
@annevk annevk deleted the annevk/shadow-open branch April 10, 2024 06:31
annevk added a commit to whatwg/html that referenced this pull request Apr 10, 2024
I missed this in review but my understanding of the discussion we had around #8867 was that the contract would be the same for open and closed shadow roots.

Tests: web-platform-tests/wpt#45624.
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 12, 2024
Per the spec change:

  whatwg/html#10260

This is a small behavior change. If `{serializableShadowRoots:true}`
and a given shadow root is `serializable`, then serialize it, even
if it is closed.

Tests should flow in from this upstream WPT change:

  web-platform-tests/wpt#45624

Bug: 41490936
Change-Id: I85f239599dda60637ff6301d1a501f9cce24a861
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5448166
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1286531}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants