-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Conversation
EWS run on previous version of this PR (hash f9ce82f) |
EWS run on previous version of this PR (hash 0946f19) |
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.
LGTM with minor nits.
Source/WebCore/dom/Element.h
Outdated
@@ -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); |
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.
nit: looks like an extra space before "attach".
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.
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)) |
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.
if (!is<HTMLTemplateElement>(*node))
would avoid an unnecessary null check.
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.
Good pint. Will do.
next = NodeTraversal::next(*node); | ||
if (!is<HTMLTemplateElement>(node)) | ||
continue; | ||
RefPtr parentElement = node->parentElement(); |
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.
Your call but this is less lines:
if (RefPtr parentElement = node->parentElement())
downcast<HTMLTemplateElement>(*node).attachAsDeclarativeShadowRootIfNeeded(*parentElement);
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.
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()); |
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.
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.
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.
Sure. Will do
EWS run on previous version of this PR (hash 00021eb) |
EWS run on current version of this PR (hash 2561ad4) |
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
to
46837e5
Compare
Committed 254964@main (46837e5): https://commits.webkit.org/254964@main Reviewed commits have been landed. Closing PR #4693 and removing active labels. |
46837e5
2561ad4
🛠 🧪 win🛠 mac-debug🧪 ios-wk2🧪 mac-AS-debug-wk2