Add getHTML() and the serializable concept #10139

mfreed7 · 2024-02-16T03:10:47Z

Add getHTML() and the serializable concept for shadow roots. See #8867 (comment) for the rough consensus on how this should work, plus the few comments below that with modifications.

At least two implementers are interested (and none opposed):
- Chromium
- WebKit
Tests are written and can be reviewed and commented upon at:
- https://wpt.fyi/results/shadow-dom/declarative/gethtml.html
Implementation bugs are filed:
- Chromium: https://crbug.com/41490936
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1886630
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=271343
MDN issue is filed:
- The getHTML(), setHTMLUnsafe(), and parseHTMLUnsafe() methods are missing mdn/content#32731
The top of this comment includes a clear commit message to use.

See the corresponding DOM spec PR here: whatwg/dom#1256

/canvas.html ( diff )
/dynamic-markup-insertion.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/interaction.html ( diff )
/parsing.html ( diff )
/scripting.html ( diff )
/semantics.html ( diff )

mfreed7 · 2024-02-16T03:15:26Z

Parsing MDN data...
Parsing...
Parse Error: (130197,76) unexpected end tag

I'm not sure what this is - I do not get any errors when I build locally, using the build.sh script. Clues appreciated.

source

domenic · 2024-02-16T03:31:55Z

I'm not sure what this is - I do not get any errors when I build locally, using the build.sh script. Clues appreciated.

This is due to whatwg/html-build#290. You have some invalid HTML around that line, but the Rust portion of the build step is fixing it up for you. PR preview doesn't run the Rust portion of the build step, so it fails.

You can try applying whatwg/html-build#291 locally. Or you can stare really hard at (130197,76) to find the extra end tag.

mfreed7 · 2024-02-16T16:10:20Z

This is due to whatwg/html-build#290. You have some invalid HTML around that line, but the Rust portion of the build step is fixing it up for you. PR preview doesn't run the Rust portion of the build step, so it fails.

Ahh, thanks. Is there a way to run the local build without the Rust, so it matches what the preview does? Generally I try to make sure things build before I put up a PR, so I was disappointed to see this failure.

You can try applying whatwg/html-build#291 locally. Or you can stare really hard at (130197,76) to find the extra end tag.

Thanks. 130197 is a blank line, and I had been staring at 130198 instead. Turns out the problem was on 130196.

mfreed7 · 2024-02-16T16:11:31Z

Parse Error: (142841,9) unexpected body end tag

Looks like I'll be uploading a bunch of patches to try to find this one now...

mfreed7 · 2024-02-16T16:15:00Z

Parse Error: (142841,9) unexpected body end tag

Looks like I'll be uploading a bunch of patches to try to find this one now...

Side note, as I manually scan for mis-nested tags, it's nice that WHATWG fought for keeping HTML and not switching to XHTML.

source

mfreed7

Thanks for the review!

source

See the conversation here: whatwg/html#10139 But the TL;DR is that the `<template>` attribute corresponding to the declarative shadow root `serializable` attribute should be named in a parallel way to other DSD-related attributes, meaning it should start with `shadowroot*`. Bug: 41490936 Change-Id: I40ea496c22147bac7dcdfb74cd7bf4a3abae8be9

source

annevk · 2024-03-18T12:50:52Z

Also:

We need a WPT PR that renames away from .tentative and corrects the naming of the shadowrootserializable attribute.
There should be an MDN issue.

mfreed7 · 2024-03-18T17:57:08Z

Also:

We need a WPT PR that renames away from .tentative and corrects the naming of the shadowrootserializable attribute.

This is in progress, here and here.

There should be an MDN issue.

Done: mdn/content#32731

The spec is landing: whatwg/html#10139 Bug: 41490936 Change-Id: Ia568d9f1adc9623298db6fd470383930fa5d9d9d

See the conversation here: whatwg/html#10139 But the TL;DR is that the `<template>` attribute corresponding to the declarative shadow root `serializable` attribute should be named in a parallel way to other DSD-related attributes, meaning it should start with `shadowroot*`. Bug: 41490936 Change-Id: I40ea496c22147bac7dcdfb74cd7bf4a3abae8be9

See the conversation here: whatwg/html#10139 But the TL;DR is that the `<template>` attribute corresponding to the declarative shadow root `serializable` attribute should be named in a parallel way to other DSD-related attributes, meaning it should start with `shadowroot*`. Bug: 41490936 Change-Id: I40ea496c22147bac7dcdfb74cd7bf4a3abae8be9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5375416 Commit-Queue: Di Zhang <dizhangg@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1274374}

lukewarlow · 2024-03-18T21:38:09Z

I can't find a way to unresolve the comment and it's not relevant to this PR anymore since it's been removed, but if Chrome is using HTMLString as a return type it's likely doing nothing more than being a DOMString, or worse case is slower.

For context HTMLString comes from Trusted Types and it's an alias for [StringContext=TrustedHTML] DOMString which does some enforcement of TT when relevant CSP is set.

source

mfreed7 · 2024-03-20T22:29:05Z

I can't find a way to unresolve the comment and it's not relevant to this PR anymore since it's been removed, but if Chrome is using HTMLString as a return type it's likely doing nothing more than being a DOMString, or worse case is slower.

For context HTMLString comes from Trusted Types and it's an alias for [StringContext=TrustedHTML] DOMString which does some enforcement of TT when relevant CSP is set.

So this PR has been changed to do HTMLString -> DOMString. But I think perhaps you're talking about the Chromium implementation which does use HTMLString? If so, can you clarify what you think should be changed? Existing serializers like innerHTML and outerHTML are supposed to return HTMLString also, right? I see that in Chromium they don't currently, but I thought I should try to return HTMLString for new things. LMK if that's wrong.

The spec is landing: whatwg/html#10139 Bug: 41490936 Change-Id: Ia568d9f1adc9623298db6fd470383930fa5d9d9d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5378423 Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1275958}

annevk · 2024-03-21T08:56:33Z

I think HTMLString is only relevant for input which is why innerHTML uses it (for its setter).

It seems this PR needs rebasing. I think we should also resolve #10211.

And where you wrote "element or shadow root" you didn't update the variable name. There might be more errors, I didn't read through it all again.

domenic

The latest changes left things in a bad state, with "DOM parsing and serialization" only containing parsing spec text plus serialization IDL, and the serialization spec text and domintro still far away in a different section.

It was explicitly not my intention to make @mfreed7 resolve this disagreement between the editors in this PR by commenting on #10139 (comment), and I'm sad that my comment there was used to delay landing this. I explicitly said I'd like to work on that as a followup.

To compensate, I took it on myself to do the remaining rearranging to get things into a hopefully-agreeable state, in 1619131.

While there I got nerd-sniped into improving the domintro, which previously didn't follow our existing conventions and didn't explain the options at all. If someone could check that I summarized the behavior correctly, that would be appreciated.

mfreed7

Thank you for making these edits! Modulo two comments, this LGTM.

source

mfreed7

LGTM

source

domenic · 2024-04-02T09:02:58Z

@annevk want to do a final check? Also I think you should merge whatwg/dom#1256 first?

source

annevk · 2024-04-02T14:28:33Z

I'm sad that my comment there was used to delay landing this.

Your opinion on this apparently differs, but I don't think delaying this by a couple days is a big deal, given that it might make blame a lot more useful. I also didn't think @mfreed7 would act on the comment given what you had written there. I wanted to wait with landing until I had some conversation with you about it. I think the changes you made make sense, though Simon just spotted a bigger problem.

Corresponding HTML PR & issue: whatwg/html#10139 & whatwg/html#8867.

mfreed7 · 2024-04-04T22:39:42Z

Thanks everyone!

…rialization, a=testonly Automatic update from web-platform-tests Add a test of the ordering of getHTML serialization This adds a test to verify that the serialized version of a shadow root: 1. has the `<template shadowroot>` first, before any children, regardless of where the initial DSD content put them. 2. has the various shadowroot* attributes in a well defined order, again regardless of where the initial DSD put them. This came from a comment on the spec PR, here: whatwg/html#10139 (comment) (Note that Github currently doesn't properly handle links to "Resolved" conversations, and the above conversation is resolved. So I guess trust me that the request was to add testing for this case!) Bug: 41490936 Change-Id: I7866b61b768f645d61142a1fd4a9490ee0732dc2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5405665 Reviewed-by: David Baron <dbaron@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1279973} -- wpt-commits: 6a072aeb9fcbaa56cab5e168cba7b706268c4d05 wpt-pr: 45415

https://bugs.webkit.org/show_bug.cgi?id=271343 rdar://125513986 Reviewed by Ryosuke Niwa. This implements whatwg/html#10139 as amended by whatwg/html#10260. This also changes the way a shadow root is serialized to avoid creating a template element. The API is guarded by DeclarativeShadowRootsSerializerAPIsEnabled just in case there is a need to disable it without removing the code. Thanks to Chris Dumez for his help with Vector<RefPtr<ShadowRoot>>. WPT tests are synchronized up to and including this commit: web-platform-tests/wpt@dd2e51d This also adjusts the copyright lines to match the commit log for the relevant files. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats.html: * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml-ordering-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml-ordering.html: Added. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml.html: Added. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml.tentative-expected.txt: Removed. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml.tentative.html: Removed. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/w3c-import.log: * LayoutTests/tests-options.json: * Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml: * Source/WebCore/CMakeLists.txt: * Source/WebCore/DerivedSources-input.xcfilelist: * Source/WebCore/DerivedSources-output.xcfilelist: * Source/WebCore/DerivedSources.make: * Source/WebCore/Headers.cmake: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/dom/Element.cpp: (WebCore::Element::getHTML const): * Source/WebCore/dom/Element.h: * Source/WebCore/dom/GetHTMLOptions.h: Added. * Source/WebCore/dom/GetHTMLOptions.idl: Added. * Source/WebCore/dom/InnerHTML.idl: * Source/WebCore/dom/ShadowRoot.cpp: (WebCore::ShadowRoot::ShadowRoot): (WebCore::ShadowRoot::getHTML const): (WebCore::ShadowRoot::cloneNodeInternal): * Source/WebCore/dom/ShadowRoot.h: * Source/WebCore/dom/ShadowRoot.idl: * Source/WebCore/dom/ShadowRootInit.h: * Source/WebCore/dom/ShadowRootInit.idl: * Source/WebCore/editing/MarkupAccumulator.cpp: (WebCore::MarkupAccumulator::MarkupAccumulator): (WebCore::MarkupAccumulator::shouldIncludeShadowRoots const): (WebCore::MarkupAccumulator::includeShadowRoot const): (WebCore::MarkupAccumulator::serializeNodesWithNamespaces): (WebCore::MarkupAccumulator::replacementElement): * Source/WebCore/editing/MarkupAccumulator.h: (WebCore::MarkupAccumulator::MarkupAccumulator): * Source/WebCore/editing/markup.cpp: (WebCore::serializeFragment): * Source/WebCore/editing/markup.h: (WebCore::serializeFragment): * Source/WebCore/html/HTMLAttributeNames.in: * Source/WebCore/html/HTMLTemplateElement.cpp: (WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded): * Source/WebCore/html/HTMLTemplateElement.idl: * Source/WebCore/html/parser/HTMLConstructionSite.cpp: (WebCore::HTMLConstructionSite::insertHTMLTemplateElement): * Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp: (WebCore::LegacyWebArchive::create): Canonical link: https://commits.webkit.org/277374@main

I2S: - https://groups.google.com/a/chromium.org/g/blink-dev/c/T4jcCXRkwB8/m/GD81-4bNAAAJ Spec: - whatwg/html#10139 - whatwg/dom#1256 Fixed: 41490936 Change-Id: Ie9b52d5d7a5aa626f1d9f96326c2d4826e11874f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5427272 Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1285896}

…rialization, a=testonly Automatic update from web-platform-tests Add a test of the ordering of getHTML serialization This adds a test to verify that the serialized version of a shadow root: 1. has the `<template shadowroot>` first, before any children, regardless of where the initial DSD content put them. 2. has the various shadowroot* attributes in a well defined order, again regardless of where the initial DSD put them. This came from a comment on the spec PR, here: whatwg/html#10139 (comment) (Note that Github currently doesn't properly handle links to "Resolved" conversations, and the above conversation is resolved. So I guess trust me that the request was to add testing for this case!) Bug: 41490936 Change-Id: I7866b61b768f645d61142a1fd4a9490ee0732dc2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5405665 Reviewed-by: David Baron <dbaron@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1279973} -- wpt-commits: 6a072aeb9fcbaa56cab5e168cba7b706268c4d05 wpt-pr: 45415

mfreed7 mentioned this pull request Feb 16, 2024

Add serializable to ShadowRoot and "attach a shadow root" whatwg/dom#1256

Merged

5 tasks

mfreed7 commented Feb 16, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

mfreed7 mentioned this pull request Feb 16, 2024

Add getInnerHTML() that optionally serializes declarative Shadow DOM #8867

Closed

annevk reviewed Mar 4, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

mfreed7 commented Mar 15, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

chromium-wpt-export-bot mentioned this pull request Mar 15, 2024

Rename serializable to shadowrootserializable web-platform-tests/wpt#45142

Merged

annevk reviewed Mar 18, 2024

View reviewed changes

mfreed7 mentioned this pull request Mar 18, 2024

The getHTML(), setHTMLUnsafe(), and parseHTMLUnsafe() methods are missing mdn/content#32731

Open

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 18, 2024

Mark getHTML() test non-tentative

8af86a5

The spec is landing: whatwg/html#10139 Bug: 41490936 Change-Id: Ia568d9f1adc9623298db6fd470383930fa5d9d9d

chromium-wpt-export-bot mentioned this pull request Mar 18, 2024

Mark getHTML() test non-tentative web-platform-tests/wpt#45175

Merged

annevk reviewed Mar 19, 2024

View reviewed changes

Mason Freed added 2 commits March 21, 2024 08:44

Serialization logic for declarative Shadow DOM

6384719

Dust this off and update

694d055

Rearrange and improve domintro

1619131

domenic approved these changes Mar 29, 2024

View reviewed changes

mfreed7 commented Mar 29, 2024

View reviewed changes

source Show resolved Hide resolved

source Show resolved Hide resolved

source Show resolved Hide resolved

Re-do domintro

eda3285

mfreed7 commented Mar 29, 2024

View reviewed changes

source Show resolved Hide resolved

Mason Freed added 2 commits March 31, 2024 08:51

Put template first

5ee46c2

Remove extra li

59b40f6

zcorpan requested changes Apr 2, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

zcorpan mentioned this pull request Apr 3, 2024

getHTML() and the serializable concept mozilla/standards-positions#1006

Closed

Allow serializing just the template

73d5bda

zcorpan approved these changes Apr 4, 2024

View reviewed changes

annevk pushed a commit to whatwg/dom that referenced this pull request Apr 4, 2024

Add serializable to ShadowRoot and "attach a shadow root"

2d890bd

Corresponding HTML PR & issue: whatwg/html#10139 & whatwg/html#8867.

annevk merged commit 6c20fe5 into whatwg:main Apr 4, 2024
2 checks passed

mfreed7 mentioned this pull request Apr 5, 2024

There have been a few changes to declarative shadow DOM mdn/content#32289

Open

annevk mentioned this pull request Apr 8, 2024

Add serializable shadow roots and getHTML() WebKit/WebKit#26975

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add getHTML() and the serializable concept #10139

Add getHTML() and the serializable concept #10139

mfreed7 commented Feb 16, 2024 •

edited by pr-preview bot

mfreed7 commented Feb 16, 2024

domenic commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 left a comment

annevk commented Mar 18, 2024

mfreed7 commented Mar 18, 2024 •

edited

lukewarlow commented Mar 18, 2024

mfreed7 commented Mar 20, 2024

annevk commented Mar 21, 2024

domenic left a comment

mfreed7 left a comment

mfreed7 left a comment

domenic commented Apr 2, 2024

annevk commented Apr 2, 2024

mfreed7 commented Apr 4, 2024

Add getHTML() and the serializable concept #10139

Add getHTML() and the serializable concept #10139

Conversation

mfreed7 commented Feb 16, 2024 • edited by pr-preview bot

mfreed7 commented Feb 16, 2024

domenic commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 commented Feb 16, 2024

mfreed7 left a comment

Choose a reason for hiding this comment

annevk commented Mar 18, 2024

mfreed7 commented Mar 18, 2024 • edited

lukewarlow commented Mar 18, 2024

mfreed7 commented Mar 20, 2024

annevk commented Mar 21, 2024

domenic left a comment

Choose a reason for hiding this comment

mfreed7 left a comment

Choose a reason for hiding this comment

mfreed7 left a comment

Choose a reason for hiding this comment

domenic commented Apr 2, 2024

annevk commented Apr 2, 2024

mfreed7 commented Apr 4, 2024

mfreed7 commented Feb 16, 2024 •

edited by pr-preview bot

mfreed7 commented Mar 18, 2024 •

edited