Skip to content
Permalink
Browse files
Fix the remaining WPT failures for imperative slot API
https://bugs.webkit.org/show_bug.cgi?id=243817

Reviewed by Chris Dumez.

This patch improves the imperative slot API implementation in WebKit by adding support for slotchange event.

* LayoutTests/fast/shadow-dom/manual-assignment-multiple-shadow-roots-expected.txt: Added.
* LayoutTests/fast/shadow-dom/manual-assignment-multiple-shadow-roots.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api-slotchange-expected.txt:
* LayoutTests/platform/win/TestExpectations:

* Source/WebCore/dom/Node.cpp:
(WebCore::Node::setManuallyAssignedSlot): Destruct the renderer when the assigned slot of this node changes.

* Source/WebCore/dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::didRemoveManuallyAssignedNode): Added.
* Source/WebCore/dom/ShadowRoot.h:

* Source/WebCore/dom/SlotAssignment.cpp:
(WebCore::NamedSlotAssignment::didRemoveManuallyAssignedNode): Added.
(WebCore::NamedSlotAssignment::willRemoveAssignedNode):
(WebCore::ManualSlotAssignment::assignedNodesForSlot):
(WebCore::ManualSlotAssignment::addSlotElementByName): Added the logic to enqueue a slotchange event.
(WebCore::ManualSlotAssignment::removeSlotElementByName): Ditto.
(WebCore::ManualSlotAssignment::slotManualAssignmentDidChange): Now notifies slots in other shadow trees
about changes in assigned nodes to enqueue slotchange event.
(WebCore::ManualSlotAssignment::didRemoveManuallyAssignedNode): Added. Gets called when one of the assigned
nodes to re-assigned to a slot in another tree.
(WebCore::ManualSlotAssignment::willRemoveAssignedNode): Added the logic to enqueue a slotchange event.

* Source/WebCore/dom/SlotAssignment.h:
(WebCore::ShadowRoot::willRemoveAssignedNode):

* Source/WebCore/html/HTMLSlotElement.cpp:
(WebCore::HTMLSlotElement::assign): Added the logic to clear the previously assigned slot.

Canonical link: https://commits.webkit.org/253359@main
  • Loading branch information
rniwa committed Aug 12, 2022
1 parent 8260ed6 commit 5473340616467e397f8dcd572c0ef84957e9ae57
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 31 deletions.
@@ -0,0 +1,6 @@

PASS slot assigned of nodes in other shadow trees
PASS orphan slot assigned of nodes in another shadow tree
PASS orphan slot assigned of nodes in multiple other shadow trees
PASS slot assigned of nodes in multiple other shadow trees

@@ -0,0 +1,137 @@
<!DOCTYPE html>
<html>
<body>
<div id="container"></div>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>

promise_test(async () => {
let logs = [];
function logger(event) { logs.push(this); }

container.innerHTML = `<div id="hostA"><div id="childA1"></div><div id="childA2"></div></div><div id="hostB"></div>`;

const shadowRootA = hostA.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotA = shadowRootA.appendChild(document.createElement('slot'));
slotA.id = 'A';
slotA.assign(childA1, childA2);
slotA.addEventListener('slotchange', logger);

await new Promise((resolve) => setTimeout(resolve, 0));

assert_array_equals(logs, [slotA]);
logs = [];

const shadowRootB = hostB.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotB = shadowRootB.appendChild(document.createElement('slot'));
slotB.id = 'B';
slotB.assign(childA1);
slotB.addEventListener('slotchange', logger);

await new Promise((resolve) => setTimeout(resolve, 0));
assert_array_equals(logs, [slotA]);
}, 'slot assigned of nodes in other shadow trees');

promise_test(async () => {
let logs = [];
function logger(event) { logs.push(this); }

container.innerHTML = `<div id="hostA"><div id="child1"></div><div id="child2"></div></div>`;

const shadowRootA = hostA.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotA = shadowRootA.appendChild(document.createElement('slot'));
slotA.id = 'A';
slotA.assign(child1, child2);
slotA.addEventListener('slotchange', logger);

await new Promise((resolve) => setTimeout(resolve, 0));

assert_array_equals(logs, [slotA]);
logs = [];

const slotB = document.createElement('slot');
slotB.assign(child2);

await new Promise((resolve) => setTimeout(resolve, 0));
assert_array_equals(logs, [slotA]);
}, 'orphan slot assigned of nodes in another shadow tree');

promise_test(async () => {
let logs = [];
function logger(event) { logs.push(this); }

container.innerHTML = `<div id="hostA"><div id="childA1"></div><div id="childA2"></div><div id="childA3"></div><div id="childA4"></div></div>
<div id="hostB"><div id="childB1"></div><div id="childB2"></div></div>
<div id="hostC"></div>`;

const shadowRootB = hostB.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotB = shadowRootB.appendChild(document.createElement('slot'));
slotB.id = 'B';
slotB.assign(childB1, childB2);
slotB.addEventListener('slotchange', logger);

const shadowRootA = hostA.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotA1 = shadowRootA.appendChild(document.createElement('slot'));
slotA1.id = 'A1';
slotA1.assign(childA1, childA2);
slotA1.addEventListener('slotchange', logger);

const slotA2 = shadowRootA.appendChild(document.createElement('slot'));
slotA2.id = 'A2';
slotA2.assign(childA3, childA4);
slotA2.addEventListener('slotchange', logger);

await new Promise((resolve) => setTimeout(resolve, 0));

assert_array_equals(logs, [slotB, slotA1, slotA2]);
logs = [];

const slotC = document.createElement('slot');
slotC.assign(childB1, childA3, childA2);

await new Promise((resolve) => setTimeout(resolve, 0));
assert_array_equals(logs, [slotB, slotA2, slotA1]);
}, 'orphan slot assigned of nodes in multiple other shadow trees');

promise_test(async () => {
let logs = [];
function logger(event) { logs.push(this); }

container.innerHTML = `<div id="hostA"><div id="childA1"></div><div id="childA2"></div><div id="childA3"></div><div id="childA4"></div></div>
<div id="hostB"><div id="childB1"></div><div id="childB2"></div></div>
<div id="hostC"></div>`;

const shadowRootB = hostB.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotB = shadowRootB.appendChild(document.createElement('slot'));
slotB.id = 'B';
slotB.assign(childB1, childB2);
slotB.addEventListener('slotchange', logger);

const shadowRootA = hostA.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotA1 = shadowRootA.appendChild(document.createElement('slot'));
slotA1.id = 'A1';
slotA1.assign(childA1, childA2);
slotA1.addEventListener('slotchange', logger);

const slotA2 = shadowRootA.appendChild(document.createElement('slot'));
slotA2.id = 'A2';
slotA2.assign(childA3, childA4);
slotA2.addEventListener('slotchange', logger);

await new Promise((resolve) => setTimeout(resolve, 0));

assert_array_equals(logs, [slotB, slotA1, slotA2]);
logs = [];

const shadowRootC = hostC.attachShadow({mode: 'closed', slotAssignment: 'manual'});
const slotC = shadowRootC.appendChild(document.createElement('slot'));
slotC.assign(childB1, childA3, childA2);

await new Promise((resolve) => setTimeout(resolve, 0));
assert_array_equals(logs, [slotB, slotA2, slotA1]);
}, 'slot assigned of nodes in multiple other shadow trees');

</script>
</body>
</html>
@@ -12,6 +12,6 @@ PASS Appending slottable to different host, it loses slot assignment. It can be
PASS Previously assigned node should not be assigned if slot moved to a new shadow root. The node is re-assigned when moved back.
PASS Assignment with the same node in parameters should be ignored, first one wins.
PASS Removing a slot from DOM resets its slottable's slot assignment.
FAIL Nodes can be assigned even if slots or nodes aren't in the same tree. assert_equals: expected Element node <slot id="s1"></slot> but got null
PASS Nodes can be assigned even if slots or nodes aren't in the same tree.
PASS Removing a node from the document does not break manually assigned slot linkage.

@@ -1,17 +1,15 @@

Harness Error (TIMEOUT), message = null

PASS slotchange event must not fire synchronously.
PASS slotchange event should not fire when assignments do not change assignedNodes.
PASS slotchange event should not fire when same node is assigned.
PASS Fire slotchange event when slot's assigned nodes changes.
PASS Fire slotchange event on previous slot and new slot when node is reassigned.
PASS Fire slotchange event on node assignment and when assigned node is removed.
PASS Fire slotchange event when order of assigned nodes changes.
TIMEOUT Fire slotchange event when assigned node is removed. Test timed out
NOTRUN Fire slotchange event when removing a slot from Shadows Root that changes its assigned nodes.
PASS Fire slotchange event when assigned node is removed.
PASS Fire slotchange event when removing a slot from Shadows Root that changes its assigned nodes.
PASS No slotchange event when adding or removing an empty slot.
PASS No slotchange event when adding another slotable.
PASS Fire slotchange event when assign node to nested slot, ensure event bubbles ups.
NOTRUN Signal a slot change should be done in tree order.
PASS Signal a slot change should be done in tree order.

@@ -3312,6 +3312,9 @@ webkit.org/b/152411 http/tests/contentdispositionattachmentsandbox/referer-heade
# Touch events are not enabled on Windows
webkit.org/b/149592 fast/shadow-dom/touch-event-ios.html [ Skip ]

# Imperative slot API isn't enabled on Windows yet.
fast/shadow-dom/manual-assignment-multiple-shadow-roots.html [ Failure ]

# The SVG -> OTF Font converter outputs 'kern' tables instead of 'GPOS' tables.
webkit.org/b/137204 fast/text/svg-font-face-with-kerning.html [ Failure ]
webkit.org/b/137204 svg/W3C-SVG-1.1/fonts-kern-01-t.svg [ Failure ]
@@ -65,6 +65,7 @@
#include "RenderBlock.h"
#include "RenderBox.h"
#include "RenderTextControl.h"
#include "RenderTreeUpdater.h"
#include "RenderView.h"
#include "SVGElement.h"
#include "ScopedEventQueue.h"
@@ -1223,6 +1224,11 @@ HTMLSlotElement* Node::manuallyAssignedSlot() const

void Node::setManuallyAssignedSlot(HTMLSlotElement* slotElement)
{
if (RefPtr element = dynamicDowncast<Element>(*this))
RenderTreeUpdater::tearDownRenderers(*element);
else if (RefPtr text = dynamicDowncast<Text>(*this))
RenderTreeUpdater::tearDownRenderer(*text);

ensureRareData().setManuallyAssignedSlot(slotElement);
}

@@ -272,6 +272,12 @@ void ShadowRoot::slotManualAssignmentDidChange(HTMLSlotElement& slot, Vector<Wea
m_slotAssignment->slotManualAssignmentDidChange(slot, previous, current, *this);
}

void ShadowRoot::didRemoveManuallyAssignedNode(HTMLSlotElement& slot, const Node& node)
{
ASSERT(m_slotAssignment);
m_slotAssignment->didRemoveManuallyAssignedNode(slot, node, *this);
}

void ShadowRoot::slotFallbackDidChange(HTMLSlotElement& slot)
{
ASSERT(&slot.rootNode() == this);
@@ -99,6 +99,7 @@ class ShadowRoot final : public DocumentFragment, public TreeScope {
void addSlotElementByName(const AtomString&, HTMLSlotElement&);
void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode& oldParentOfRemovedTree);
void slotManualAssignmentDidChange(HTMLSlotElement&, Vector<WeakPtr<Node>>& previous, Vector<WeakPtr<Node>>& current);
void didRemoveManuallyAssignedNode(HTMLSlotElement&, const Node&);
void slotFallbackDidChange(HTMLSlotElement&);
void resolveSlotsBeforeNodeInsertionOrRemoval();
void willRemoveAllChildren(ContainerNode&);
@@ -275,6 +275,10 @@ void NamedSlotAssignment::slotManualAssignmentDidChange(HTMLSlotElement&, Vector
{
}

void NamedSlotAssignment::didRemoveManuallyAssignedNode(HTMLSlotElement&, const Node&, ShadowRoot&)
{
}

void NamedSlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
{
if (shadowRoot.mode() == ShadowRootMode::UserAgent)
@@ -352,7 +356,7 @@ const Vector<WeakPtr<Node>>* NamedSlotAssignment::assignedNodesForSlot(const HTM
return &slot->assignedNodes;
}

void NamedSlotAssignment::willRemoveAssignedNode(const Node& node)
void NamedSlotAssignment::willRemoveAssignedNode(const Node& node, ShadowRoot&)
{
if (!m_slotAssignmentsIsValid)
return;
@@ -453,36 +457,45 @@ void ManualSlotAssignment::renameSlotElement(HTMLSlotElement&, const AtomString&
{
}

void ManualSlotAssignment::addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&)
void ManualSlotAssignment::addSlotElementByName(const AtomString&, HTMLSlotElement& slot, ShadowRoot& shadowRoot)
{
if (!m_slotElementCount)
shadowRoot.host()->setHasShadowRootContainingSlots(true);
++m_slotElementCount;
++m_slottableVersion;
if (assignedNodesForSlot(slot, shadowRoot))
slot.enqueueSlotChangeEvent();
}

void ManualSlotAssignment::removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode*, ShadowRoot&)
void ManualSlotAssignment::removeSlotElementByName(const AtomString&, HTMLSlotElement& slot, ContainerNode*, ShadowRoot& shadowRoot)
{
RELEASE_ASSERT(m_slotElementCount);
--m_slotElementCount;
++m_slottableVersion;
for (auto& node : slot.manuallyAssignedNodes()) {
if (node && node->parentNode() == shadowRoot.host()) {
slot.enqueueSlotChangeEvent();
break;
}
}
}

void ManualSlotAssignment::slotManualAssignmentDidChange(HTMLSlotElement& slot, Vector<WeakPtr<Node>>& previous, Vector<WeakPtr<Node>>& current, ShadowRoot& shadowRoot)
{
for (auto& node : previous)
node->setManuallyAssignedSlot(nullptr);

auto effectivePrevious = effectiveAssignedNodes(shadowRoot, previous);
for (auto& node : effectivePrevious) {
if (RefPtr element = dynamicDowncast<Element>(*node))
RenderTreeUpdater::tearDownRenderers(*element);
else if (RefPtr text = dynamicDowncast<Text>(*node))
RenderTreeUpdater::tearDownRenderer(*text);
}

HashSet<Ref<HTMLSlotElement>> affectedSlots;
for (auto& node : current) {
if (auto* previousSlot = node->manuallyAssignedSlot()) {
RELEASE_ASSERT(previousSlot != &slot); // Duplicate entries must have been removed in HTMLSlotElement::assign.
previousSlot->removeManuallyAssignedNode(*node);
if (previousSlot->isInShadowTree() && previousSlot->containingShadowRoot() == slot.containingShadowRoot())
RefPtr protectedNode = node.get();
if (RefPtr previousSlot = protectedNode->manuallyAssignedSlot()) {
previousSlot->removeManuallyAssignedNode(*protectedNode);
RefPtr shadowRootOfPreviousSlot = previousSlot->containingShadowRoot();
// FIXME: It's odd that slotchange event is enqueued in the tree order for other slots in the same shadow tree.
if (shadowRootOfPreviousSlot == &shadowRoot && protectedNode->parentNode() == shadowRoot.host())
affectedSlots.add(*previousSlot);
else if (shadowRootOfPreviousSlot && shadowRootOfPreviousSlot->host() == protectedNode->parentNode())
shadowRootOfPreviousSlot->didRemoveManuallyAssignedNode(*previousSlot, *protectedNode);
}
node->setManuallyAssignedSlot(&slot);
protectedNode->setManuallyAssignedSlot(&slot);
}

++m_slottableVersion;
@@ -516,6 +529,16 @@ void ManualSlotAssignment::slotManualAssignmentDidChange(HTMLSlotElement& slot,
}
}

void ManualSlotAssignment::didRemoveManuallyAssignedNode(HTMLSlotElement& slot, const Node& node, ShadowRoot& shadowRoot)
{
ASSERT(slot.containingShadowRoot() == &shadowRoot);
ASSERT_UNUSED(node, node.parentNode() == shadowRoot.host());
++m_slottableVersion;
RenderTreeUpdater::tearDownRenderersAfterSlotChange(*shadowRoot.host());
shadowRoot.host()->invalidateStyleForSubtree();
slot.enqueueSlotChangeEvent();
}

void ManualSlotAssignment::slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&)
{
++m_slottableVersion;
@@ -530,9 +553,11 @@ void ManualSlotAssignment::hostChildElementDidChangeSlotAttribute(Element&, cons
{
}

void ManualSlotAssignment::willRemoveAssignedNode(const Node&)
void ManualSlotAssignment::willRemoveAssignedNode(const Node& node, ShadowRoot& shadowRoot)
{
++m_slottableVersion;
if (RefPtr slot = node.assignedSlot(); slot && slot->containingShadowRoot() == &shadowRoot)
slot->enqueueSlotChangeEvent();
}

void ManualSlotAssignment::didRemoveAllChildrenOfShadowHost(ShadowRoot&)
@@ -56,12 +56,13 @@ class SlotAssignment {
virtual void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&) = 0;
virtual void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot&) = 0;
virtual void slotManualAssignmentDidChange(HTMLSlotElement&, Vector<WeakPtr<Node>>& previous, Vector<WeakPtr<Node>>& current, ShadowRoot&) = 0;
virtual void didRemoveManuallyAssignedNode(HTMLSlotElement&, const Node&, ShadowRoot&) = 0;
virtual void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&) = 0;

virtual void hostChildElementDidChange(const Element&, ShadowRoot&) = 0;
virtual void hostChildElementDidChangeSlotAttribute(Element&, const AtomString& oldValue, const AtomString& newValue, ShadowRoot&) = 0;

virtual void willRemoveAssignedNode(const Node&) = 0;
virtual void willRemoveAssignedNode(const Node&, ShadowRoot&) = 0;
virtual void didRemoveAllChildrenOfShadowHost(ShadowRoot&) = 0;
virtual void didMutateTextNodesOfShadowHost(ShadowRoot&) = 0;

@@ -90,10 +91,11 @@ class NamedSlotAssignment : public SlotAssignment {
void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&) final;
void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode* oldParentOfRemovedTreeForRemoval, ShadowRoot&) final;
void slotManualAssignmentDidChange(HTMLSlotElement&, Vector<WeakPtr<Node>>& previous, Vector<WeakPtr<Node>>& current, ShadowRoot&) final;
void didRemoveManuallyAssignedNode(HTMLSlotElement&, const Node&, ShadowRoot&) final;
void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&) final;

const Vector<WeakPtr<Node>>* assignedNodesForSlot(const HTMLSlotElement&, ShadowRoot&) final;
void willRemoveAssignedNode(const Node&) final;
void willRemoveAssignedNode(const Node&, ShadowRoot&) final;

void didRemoveAllChildrenOfShadowHost(ShadowRoot&) final;
void didMutateTextNodesOfShadowHost(ShadowRoot&) final;
@@ -148,12 +150,13 @@ class ManualSlotAssignment : public SlotAssignment {
void addSlotElementByName(const AtomString&, HTMLSlotElement&, ShadowRoot&) final;
void removeSlotElementByName(const AtomString&, HTMLSlotElement&, ContainerNode*, ShadowRoot&) final;
void slotManualAssignmentDidChange(HTMLSlotElement&, Vector<WeakPtr<Node>>& previous, Vector<WeakPtr<Node>>& current, ShadowRoot&) final;
void didRemoveManuallyAssignedNode(HTMLSlotElement&, const Node&, ShadowRoot&) final;
void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&) final;

void hostChildElementDidChange(const Element&, ShadowRoot&) final;
void hostChildElementDidChangeSlotAttribute(Element&, const AtomString&, const AtomString&, ShadowRoot&) final;

void willRemoveAssignedNode(const Node&) final;
void willRemoveAssignedNode(const Node&, ShadowRoot&) final;
void didRemoveAllChildrenOfShadowHost(ShadowRoot&) final;
void didMutateTextNodesOfShadowHost(ShadowRoot&) final;

@@ -164,6 +167,7 @@ class ManualSlotAssignment : public SlotAssignment {
};
WeakHashMap<HTMLSlotElement, Slot> m_slots;
uint64_t m_slottableVersion { 0 };
unsigned m_slotElementCount { 0 };
};

inline void SlotAssignment::resolveSlotsBeforeNodeInsertionOrRemoval()
@@ -218,7 +222,7 @@ inline void ShadowRoot::hostChildElementDidChangeSlotAttribute(Element& element,
inline void ShadowRoot::willRemoveAssignedNode(const Node& node)
{
if (m_slotAssignment)
m_slotAssignment->willRemoveAssignedNode(node);
m_slotAssignment->willRemoveAssignedNode(node, *this);
}

} // namespace WebCore

0 comments on commit 5473340

Please sign in to comment.