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

Prototype declarative shadow DOM #4693

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Sep 26, 2022

46837e5

Prototype declarative shadow DOM
https://bugs.webkit.org/show_bug.cgi?id=245556

Reviewed by Chris Dumez.

Implement the declarative shadow DOM an experimental feature.

See whatwg/html#5465 and whatwg/dom#892

There are a few differences between what's being proposed and what we implement:
1. getInnerHTML method is not added; we've given quite a few feedback on this method, and it's nowhere near ready for implementation.
2. Declarative shadow DOMs inside another template element doesn't automatically get converted to a shadow tree,
and there is no special treatment of declarative shadow DOM when cloning Nodes.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popups/popup-light-dismiss.tentative-expected.txt: Rebaselined.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-attachment.tentative-expected.txt:
Rebaselined the test now that we pass more test cases.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-basic.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-opt-in.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/innerhtml-before-closing-tag.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/move-template-before-closing-tag-expected.txt: Removed.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/move-template-before-closing-tag.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/script-access.tentative-expected.txt: Rebaselined.

* Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml: Added a runtime flag.

* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:

* Source/WebCore/dom/DOMImplementation.cpp:
(WebCore::createXMLDocument): Disable declarative shadow DOM in a document created by DOMImplementation.createDocument.
(WebCore::DOMImplementation::createDocument): Ditto.
(WebCore::DOMImplementation::createHTMLDocument): Ditto.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::Document): Initialize newly introduced m_parserContentPolicy with DefaultParserContentPolicy.
(WebCore::Document::createParser): Specify m_parserContentPolicy as an argument to XMLDocumentParser::create.

* Source/WebCore/dom/Document.h:
(WebCore::Document::parserContentPolicy const): Added.
(WebCore::Document::setParserContentPolicy): Added.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::attachShadow): Returns the shadow root of declarative shadow DOM if one is available. This happens exactly once
since this function then clears the flag from ShadowRoot.
(WebCore::Element::attachDeclarativeShadow): Added.

* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/FragmentScriptingPermission.h:
(WebCore::ParserContentPolicy): Added AllowDeclarativeShadowDOM.

* Source/WebCore/dom/ScriptableDocumentParser.h:
(WebCore::ScriptableDocumentParser::parserContentPolicy const): Made this const.
(WebCore::ScriptableDocumentParser::setParserContentPolicy): Added.
(WebCore::ScriptableDocumentParser::ScriptableDocumentParser): Added

* Source/WebCore/dom/ShadowRoot.h:
(WebCore::ShadowRoot::isDeclarativeShadowRoot): Added.
(WebCore::ShadowRoot::setIsDeclarativeShadowRoot): Added.

* Source/WebCore/html/HTMLAttributeNames.in:

* Source/WebCore/html/HTMLDocument.cpp:
(WebCore::HTMLDocument::createParser): Pass in the parser content policy.

* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded): Added. Implements the main logic of declarative shadow DOM.

* Source/WebCore/html/HTMLTemplateElement.h:
* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::attachDeclarativeShadowRootIfNeeded): Added.

* Source/WebCore/html/parser/HTMLConstructionSite.h:

* Source/WebCore/html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::HTMLDocumentParser): Takes OptionSet<ParserContentPolicy> as an argument.
(WebCore::HTMLDocumentParser::create): Ditto.

* Source/WebCore/html/parser/HTMLDocumentParser.h:

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processTemplateEndTag): Attach the template content as the declarative shadow DOM if applicable.

* Source/WebCore/xml/DOMParser.cpp:
(WebCore::DOMParser::parseFromString): Added ParseFromStringOptions as an argument.

* Source/WebCore/xml/DOMParser.h:
* Source/WebCore/xml/DOMParser.idl:

* Source/WebCore/xml/ParseFromStringOptions.h: Added.
* Source/WebCore/xml/ParseFromStringOptions.idl: Added.

* Source/WebCore/xml/XMLHttpRequest.cpp:
(XMLHttpRequest::responseXML): Disable declarative shadow DOM in the response document.

* Source/WebCore/xml/parser/XMLDocumentParser.h:
(WebCore::XMLDocumentParser::create): Takes OptionSet<ParserContentPolicy> as an argument.

* Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::XMLDocumentParser::XMLDocumentParser): Ditto.

Canonical link: https://commits.webkit.org/254964@main

2561ad4

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac ❌ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk1 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 jsc-mips-tests

@rniwa rniwa requested a review from cdumez as a code owner September 26, 2022 03:03
@rniwa rniwa self-assigned this Sep 26, 2022
@rniwa rniwa added DOM For bugs specific to XML/HTML DOM elements (including parsing). WebKit Nightly Build labels Sep 26, 2022
@rniwa rniwa requested a review from anttijk September 26, 2022 03:04
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 26, 2022
@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Sep 27, 2022
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 0946f19. Live statuses available at the PR page, #4693

Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM with minor nits.

@@ -334,6 +334,7 @@ class Element : public ContainerNode {
ShadowRoot* shadowRootForBindings(JSC::JSGlobalObject&) const;

WEBCORE_EXPORT ExceptionOr<ShadowRoot&> attachShadow(const ShadowRootInit&);
ExceptionOr<ShadowRoot&> attachDeclarativeShadow(ShadowRootMode, bool delegatesFocus);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: looks like an extra space before "attach".

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh oops, will fix.

auto importedContent = document().importNode(content(), /* deep */ true).releaseReturnValue();
for (RefPtr<Node> node = NodeTraversal::next(importedContent), next; node; node = next) {
next = NodeTraversal::next(*node);
if (!is<HTMLTemplateElement>(node))
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!is<HTMLTemplateElement>(*node)) would avoid an unnecessary null check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good pint. Will do.

next = NodeTraversal::next(*node);
if (!is<HTMLTemplateElement>(node))
continue;
RefPtr parentElement = node->parentElement();
Copy link
Contributor

Choose a reason for hiding this comment

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

Your call but this is less lines:

if (RefPtr parentElement = node->parentElement())
    downcast<HTMLTemplateElement>(*node).attachAsDeclarativeShadowRootIfNeeded(*parentElement);

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, let's go with that.

@@ -900,10 +901,21 @@ bool HTMLTreeBuilder::processTemplateEndTag(AtomHTMLToken&& token)
m_tree.generateImpliedEndTags();
if (m_tree.currentStackItem().elementName() != HTML::template_)
parseError(token);
m_tree.openElements().popUntilPopped(HTML::template_);
m_tree.openElements().popUntil(HTML::template_);
RefPtr templateElement = dynamicDowncast<HTMLTemplateElement>(m_tree.openElements().top());
Copy link
Contributor

Choose a reason for hiding this comment

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

So this code does a release type check here and then a release null check on the next line. I think the following would be slightly more efficient:

RELEASE_ASSERT(is<HTMLTemplateElement>(m_tree.openElements().top()); // Release type check.
Ref templateElement = downcast<HTMLTemplateElement>(*m_tree.openElements().top()); // no release check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Will do

@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 00021eb. Live statuses available at the PR page, #4693

@rniwa rniwa added the merge-queue Applied to send a pull request to merge-queue label Sep 28, 2022
https://bugs.webkit.org/show_bug.cgi?id=245556

Reviewed by Chris Dumez.

Implement the declarative shadow DOM an experimental feature.

See whatwg/html#5465 and whatwg/dom#892

There are a few differences between what's being proposed and what we implement:
1. getInnerHTML method is not added; we've given quite a few feedback on this method, and it's nowhere near ready for implementation.
2. Declarative shadow DOMs inside another template element doesn't automatically get converted to a shadow tree,
and there is no special treatment of declarative shadow DOM when cloning Nodes.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popups/popup-light-dismiss.tentative-expected.txt: Rebaselined.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-attachment.tentative-expected.txt:
Rebaselined the test now that we pass more test cases.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-basic.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-opt-in.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/innerhtml-before-closing-tag.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/move-template-before-closing-tag-expected.txt: Removed.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/move-template-before-closing-tag.tentative-expected.txt: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/script-access.tentative-expected.txt: Rebaselined.

* Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml: Added a runtime flag.

* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:

* Source/WebCore/dom/DOMImplementation.cpp:
(WebCore::createXMLDocument): Disable declarative shadow DOM in a document created by DOMImplementation.createDocument.
(WebCore::DOMImplementation::createDocument): Ditto.
(WebCore::DOMImplementation::createHTMLDocument): Ditto.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::Document): Initialize newly introduced m_parserContentPolicy with DefaultParserContentPolicy.
(WebCore::Document::createParser): Specify m_parserContentPolicy as an argument to XMLDocumentParser::create.

* Source/WebCore/dom/Document.h:
(WebCore::Document::parserContentPolicy const): Added.
(WebCore::Document::setParserContentPolicy): Added.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::attachShadow): Returns the shadow root of declarative shadow DOM if one is available. This happens exactly once
since this function then clears the flag from ShadowRoot.
(WebCore::Element::attachDeclarativeShadow): Added.

* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/FragmentScriptingPermission.h:
(WebCore::ParserContentPolicy): Added AllowDeclarativeShadowDOM.

* Source/WebCore/dom/ScriptableDocumentParser.h:
(WebCore::ScriptableDocumentParser::parserContentPolicy const): Made this const.
(WebCore::ScriptableDocumentParser::setParserContentPolicy): Added.
(WebCore::ScriptableDocumentParser::ScriptableDocumentParser): Added

* Source/WebCore/dom/ShadowRoot.h:
(WebCore::ShadowRoot::isDeclarativeShadowRoot): Added.
(WebCore::ShadowRoot::setIsDeclarativeShadowRoot): Added.

* Source/WebCore/html/HTMLAttributeNames.in:

* Source/WebCore/html/HTMLDocument.cpp:
(WebCore::HTMLDocument::createParser): Pass in the parser content policy.

* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded): Added. Implements the main logic of declarative shadow DOM.

* Source/WebCore/html/HTMLTemplateElement.h:
* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::attachDeclarativeShadowRootIfNeeded): Added.

* Source/WebCore/html/parser/HTMLConstructionSite.h:

* Source/WebCore/html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::HTMLDocumentParser): Takes OptionSet<ParserContentPolicy> as an argument.
(WebCore::HTMLDocumentParser::create): Ditto.

* Source/WebCore/html/parser/HTMLDocumentParser.h:

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processTemplateEndTag): Attach the template content as the declarative shadow DOM if applicable.

* Source/WebCore/xml/DOMParser.cpp:
(WebCore::DOMParser::parseFromString): Added ParseFromStringOptions as an argument.

* Source/WebCore/xml/DOMParser.h:
* Source/WebCore/xml/DOMParser.idl:

* Source/WebCore/xml/ParseFromStringOptions.h: Added.
* Source/WebCore/xml/ParseFromStringOptions.idl: Added.

* Source/WebCore/xml/XMLHttpRequest.cpp:
(XMLHttpRequest::responseXML): Disable declarative shadow DOM in the response document.

* Source/WebCore/xml/parser/XMLDocumentParser.h:
(WebCore::XMLDocumentParser::create): Takes OptionSet<ParserContentPolicy> as an argument.

* Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::XMLDocumentParser::XMLDocumentParser): Ditto.

Canonical link: https://commits.webkit.org/254964@main
@webkit-commit-queue
Copy link
Collaborator

Committed 254964@main (46837e5): https://commits.webkit.org/254964@main

Reviewed commits have been landed. Closing PR #4693 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 46837e5 into WebKit:main Sep 28, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 28, 2022
@rniwa rniwa deleted the fix245556 branch February 2, 2023 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants