Skip to content

Commit

Permalink
Add shadowrootclonable and align with declarative shadow root changes
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=269361

Reviewed by Ryosuke Niwa.

This makes the following changes:

- Adds the new shadowrootclonable attribute to opt into a declarative
  shadow root being clonable.
- As a result, declarative shadow roots are no longer clonable by
  default. Web developers will have to explicitly opt in.
- When attachShadow() is called on a shadow host with an existing
  declarative tree, throw if mode is a mismatch.
- In attachShadow() throw first for mode being set to "user-agent" as
  this is to be caught by the binding layer in theory.
- And finally, only attach a declarative shadow root successfully for
  the first template element.

New tests are upstreamed here:
web-platform-tests/wpt#44568

Specification changes are here (not all have landed yet as various nits
are still being addressed, but all have agreement):

- whatwg/html#10117
- whatwg/html#10069
- whatwg/dom#1246

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/scroll-timeline-name-shadow.html:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/view-timeline-name-shadow.html:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-attachment-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-basic-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-2-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-2.html: Added.
* 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/w3c-import.log:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/shadow-root-clonable-expected.txt:
* Source/WebCore/dom/Element.cpp:
* Source/WebCore/dom/Element.h:
* Source/WebCore/html/HTMLAttributeNames.in:
* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded):
* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::insertHTMLTemplateElement):

Canonical link: https://commits.webkit.org/274727@main
  • Loading branch information
annevk committed Feb 15, 2024
1 parent b60f7f2 commit 79d2dec
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 254 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/web-animations/testcommon.js"></script>
<script src="/resources/declarative-shadow-dom-polyfill.js"></script>

<main id=main></main>
<script>
Expand All @@ -15,10 +14,6 @@
main.append(template.content.cloneNode(true));
main.offsetTop;
}

setup(() => {
polyfill_declarative_shadow_dom(document);
});
</script>
<style>
@keyframes anim {
Expand All @@ -39,7 +34,7 @@
</style>
<div class=scroller>
<div class=scroller>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
:host {
scroll-timeline: --timeline y;
Expand Down Expand Up @@ -76,7 +71,7 @@
}
</style>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
::slotted(.scroller) {
scroll-timeline: --timeline y;
Expand Down Expand Up @@ -113,7 +108,7 @@
}
</style>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
/* Not using 'anim' at document scope, due to https://crbug.com/1334534 */
@keyframes anim2 {
Expand Down Expand Up @@ -157,7 +152,7 @@
</style>
<div class=scroller>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
div {
scroll-timeline: --timeline y;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/web-animations/testcommon.js"></script>
<script src="/resources/declarative-shadow-dom-polyfill.js"></script>

<main id=main></main>
<script>
Expand All @@ -15,10 +14,6 @@
main.append(template.content.cloneNode(true));
main.offsetTop;
}

setup(() => {
polyfill_declarative_shadow_dom(document);
});
</script>
<style>
@keyframes anim {
Expand All @@ -41,7 +36,7 @@
<div class=scroller>
<div>
<div class=target>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
:host {
view-timeline: --timeline y;
Expand Down Expand Up @@ -78,7 +73,7 @@
</style>
<div class=scroller>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
::slotted(.target) {
view-timeline: --timeline y;
Expand Down Expand Up @@ -114,7 +109,7 @@
}
</style>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
/* Not using 'anim' at document scope, due to https://crbug.com/1334534 */
@keyframes anim2 {
Expand Down Expand Up @@ -158,7 +153,7 @@
</style>
<div class=scroller>
<div class=host>
<template shadowrootmode=open>
<template shadowrootmode=open shadowrootclonable>
<style>
div {
view-timeline: --timeline y;
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ PASS Declarative Shadow DOM: Invalid shadow root attribute
PASS Declarative Shadow DOM: Closed shadow root attribute
PASS Declarative Shadow DOM: Missing closing tag
PASS Declarative Shadow DOM: delegates focus attribute
FAIL Declarative Shadow DOM: clonable attribute assert_false: clonable should be false without the shadowrootclonable attribute expected false got true
FAIL Declarative Shadow DOM: Multiple roots assert_true: The second (duplicate) template should be left in the DOM expected true got false
PASS Declarative Shadow DOM: clonable attribute
PASS Declarative Shadow DOM: Multiple roots
PASS Declarative Shadow DOM: template containing declarative shadow root (with shadowrootclonable)
PASS Declarative Shadow DOM: template containing (deeply nested) declarative shadow root
PASS Declarative Shadow DOM: template containing a template containing declarative shadow root
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Repeated declarative shadow roots keep only the first

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!DOCTYPE html>
<title>Duplicate declarative shadow trees</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>

<div id=multiple1>
<template shadowrootmode=open>1</template>
<template shadowrootmode=open>2</template>
<template shadowrootmode=open>3</template>
</div>

<div id=multiple2>
<template shadowrootmode=closed>1</template>
<template shadowrootmode=closed>2</template>
<template shadowrootmode=open>3</template>
</div>

<script>
test((t) => {
t.add_cleanup(() => {
multiple1.remove();
multiple2.remove();
});
let shadow = multiple1.shadowRoot;
assert_true(!!shadow,'Remaining shadow root should be open');
assert_equals(shadow.textContent,"1");
assert_equals(multiple1.childElementCount, 2);
assert_equals(multiple1.firstElementChild.content.textContent, "2");
assert_equals(multiple1.lastElementChild.content.textContent, "3");
shadow = multiple2.shadowRoot;
assert_false(!!shadow,'Remaining shadow root should be closed');
assert_equals(multiple2.childElementCount, 2);
assert_equals(multiple2.firstElementChild.content.textContent, "2");
assert_equals(multiple2.lastElementChild.content.textContent, "3");
},'Repeated declarative shadow roots keep only the first');
</script>
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@

FAIL Repeated declarative shadow roots keep only the first assert_equals: expected "Open" but got "Closed"
FAIL Calling attachShadow() on declarative shadow root must match type assert_throws_dom: Mismatched shadow root type should throw function "() => {
open1.attachShadow({mode: "closed"});
}" did not throw
FAIL Calling attachShadow() on declarative shadow root must match all parameters assert_throws_dom: Mismatched shadow root type should throw function "() => {
open2.attachShadow({mode: "closed", delegatesFocus: true, slotAssignment: "named", clonable: true});
}" did not throw
PASS Repeated declarative shadow roots keep only the first
PASS Calling attachShadow() on declarative shadow root must match type
PASS Calling attachShadow() on declarative shadow root must match all parameters

Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,19 @@

<script>
test((t) => {
t.add_cleanup(() => {
multiple1.remove();
multiple2.remove();
});
let shadow = multiple1.shadowRoot;
assert_true(!!shadow,'Remaining shadow root should be open');
assert_equals(shadow.textContent,"Open");
assert_equals(multiple1.childElementCount, 1);
assert_equals(multiple1.firstElementChild.shadowRootMode, "closed");
shadow = multiple2.shadowRoot;
assert_false(!!shadow,'Remaining shadow root should be closed');
multiple1.remove(); // Cleanup
multiple2.remove();
assert_equals(multiple2.childElementCount, 1);
assert_equals(multiple2.firstElementChild.shadowRootMode, "open");
},'Repeated declarative shadow roots keep only the first');
</script>

Expand Down Expand Up @@ -59,20 +65,16 @@
test((t) => {
assert_throws_dom("NotSupportedError",() => {
open2.attachShadow({mode: "closed", delegatesFocus: true, slotAssignment: "named", clonable: true});
},'Mismatched shadow root type should throw');
assert_throws_dom("NotSupportedError",() => {
open2.attachShadow({mode: "open", delegatesFocus: false, slotAssignment: "named", clonable: true});
},'Mismatched shadow root delegatesFocus should throw');
assert_throws_dom("NotSupportedError",() => {
open2.attachShadow({mode: "open", delegatesFocus: true, slotAssignment: "manual", clonable: true});
},'Mismatched shadow root slotAssignment should throw');
assert_throws_dom("NotSupportedError",() => {
open2.attachShadow({mode: "open", delegatesFocus: true, slotAssignment: "named", clonable: false});
},'Mismatched shadow root clonable should throw');
},'Mismatched shadow root mode should throw');

const initialShadow = open2.shadowRoot;
const shadow = open2.attachShadow({mode: "open", delegatesFocus: true, slotAssignment: "named", clonable: true}); // Shouldn't throw
assert_equals(shadow,initialShadow,'Same shadow should be returned');
assert_equals(shadow.textContent,'','Shadow should be empty');

assert_throws_dom("NotSupportedError",() => {
open2.attachShadow({mode: "open"});
},'Invoking attachShadow() on a non-declarative shadow root should throw');

},'Calling attachShadow() on declarative shadow root must match all parameters');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ List of files:
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-available-to-element-internals.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-basic.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-opt-in.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-2.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-with-disabled-shadow.html
/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/gethtml.tentative.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
FAIL attachShadow with clonable: true null is not an object (evaluating 'shallowClonedRoot.clonable')
PASS attachShadow with clonable: false
PASS attachShadow with clonable: undefined
FAIL declarative shadow roots do *not* get clonable: true automatically assert_false: clonable is *not* automatically true for declarative shadow root expected false got true
PASS declarative shadow roots do *not* get clonable: true automatically
PASS declarative shadow roots can opt in to clonable with shadowrootclonable
FAIL declarative shadow roots inside templates do *not* get cloned automatically assert_true: no shadow root gets cloned expected true got false
PASS declarative shadow roots inside templates do *not* get cloned automatically

12 changes: 8 additions & 4 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2946,19 +2946,21 @@ static bool canAttachAuthorShadowRoot(const Element& element)

ExceptionOr<ShadowRoot&> Element::attachShadow(const ShadowRootInit& init)
{
if (init.mode == ShadowRootMode::UserAgent)
return Exception { ExceptionCode::TypeError };
if (!canAttachAuthorShadowRoot(*this))
return Exception { ExceptionCode::NotSupportedError };
if (RefPtr shadowRoot = this->shadowRoot()) {
if (shadowRoot->isDeclarativeShadowRoot()) {
if (init.mode != shadowRoot->mode())
return Exception { ExceptionCode::NotSupportedError };
ChildListMutationScope mutation(*shadowRoot);
shadowRoot->removeChildren();
shadowRoot->setIsDeclarativeShadowRoot(false);
return *shadowRoot;
}
return Exception { ExceptionCode::NotSupportedError };
}
if (init.mode == ShadowRootMode::UserAgent)
return Exception { ExceptionCode::TypeError };
Ref shadow = ShadowRoot::create(document(), init.mode, init.slotAssignment,
init.delegatesFocus ? ShadowRoot::DelegatesFocus::Yes : ShadowRoot::DelegatesFocus::No,
init.clonable ? ShadowRoot::Clonable::Yes : ShadowRoot::Clonable::No,
Expand All @@ -2967,9 +2969,11 @@ ExceptionOr<ShadowRoot&> Element::attachShadow(const ShadowRootInit& init)
return shadow.get();
}

ExceptionOr<ShadowRoot&> Element::attachDeclarativeShadow(ShadowRootMode mode, bool delegatesFocus)
ExceptionOr<ShadowRoot&> Element::attachDeclarativeShadow(ShadowRootMode mode, bool delegatesFocus, bool clonable)
{
auto exceptionOrShadowRoot = attachShadow({ mode, delegatesFocus, /* clonable */ true });
if (this->shadowRoot())
return Exception { ExceptionCode::NotSupportedError };
auto exceptionOrShadowRoot = attachShadow({ mode, delegatesFocus, clonable });
if (exceptionOrShadowRoot.hasException())
return exceptionOrShadowRoot.releaseException();
Ref shadowRoot = exceptionOrShadowRoot.releaseReturnValue();
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/dom/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ class Element : public ContainerNode {
RefPtr<ShadowRoot> shadowRootForBindings(JSC::JSGlobalObject&) const;

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

ShadowRoot* userAgentShadowRoot() const;
RefPtr<ShadowRoot> protectedUserAgentShadowRoot() const;
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/html/HTMLAttributeNames.in
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,9 @@ scrolldelay
scrolling
select
selected
shadowrootmode
shadowrootclonable
shadowrootdelegatesfocus
shadowrootmode
shape
size
sizes
Expand Down
6 changes: 4 additions & 2 deletions Source/WebCore/html/HTMLTemplateElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,11 @@ void HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded(Element& host)
if (!mode)
return;

bool delegatesFocus = hasAttributeWithoutSynchronization(HTMLNames::shadowrootdelegatesfocusAttr);
auto delegatesFocus = hasAttributeWithoutSynchronization(HTMLNames::shadowrootdelegatesfocusAttr);

auto exceptionOrShadowRoot = host.attachDeclarativeShadow(*mode, delegatesFocus);
auto clonable = hasAttributeWithoutSynchronization(HTMLNames::shadowrootclonableAttr);

auto exceptionOrShadowRoot = host.attachDeclarativeShadow(*mode, delegatesFocus, clonable);
if (exceptionOrShadowRoot.hasException())
return;

Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/html/parser/HTMLConstructionSite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ void HTMLConstructionSite::insertHTMLTemplateElement(AtomHTMLToken&& token)
{
if (document().settings().declarativeShadowRootsEnabled() && m_parserContentPolicy.contains(ParserContentPolicy::AllowDeclarativeShadowRoots)) {
std::optional<ShadowRootMode> mode;
bool clonable = false;
bool delegatesFocus = false;
for (auto& attribute : token.attributes()) {
if (attribute.name() == HTMLNames::shadowrootmodeAttr) {
Expand All @@ -553,9 +554,11 @@ void HTMLConstructionSite::insertHTMLTemplateElement(AtomHTMLToken&& token)
mode = ShadowRootMode::Open;
} else if (attribute.name() == HTMLNames::shadowrootdelegatesfocusAttr)
delegatesFocus = true;
else if (attribute.name() == HTMLNames::shadowrootclonableAttr)
clonable = true;
}
if (mode && is<Element>(currentNode())) {
auto exceptionOrShadowRoot = currentElement().attachDeclarativeShadow(*mode, delegatesFocus);
auto exceptionOrShadowRoot = currentElement().attachDeclarativeShadow(*mode, delegatesFocus, clonable);
if (!exceptionOrShadowRoot.hasException()) {
Ref shadowRoot = exceptionOrShadowRoot.releaseReturnValue();
auto element = createHTMLElement(token);
Expand Down

0 comments on commit 79d2dec

Please sign in to comment.