Change the argument type of HTMLSlotElement.assign() into (Element or…

… Text) https://bugs.webkit.org/show_bug.cgi?id=243815 Reviewed by Ryosuke Niwa. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api.html: * Source/WebCore/html/HTMLSlotElement.cpp: (WebCore::HTMLSlotElement::assign): * Source/WebCore/html/HTMLSlotElement.h: * Source/WebCore/html/HTMLSlotElement.idl: Canonical link: https://commits.webkit.org/253396@main
WebKit · Aug 13, 2022 · 48034229ee50a9abebeb9837701560cb2ad448a4 · 4803422
1 parent e2b2791
commit 48034229ee50a9abebeb9837701560cb2ad448a4
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 9 deletions.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api-expected.txt
@@ -14,4 +14,5 @@ PASS Assignment with the same node in parameters should be ignored, first one wi
 PASS Removing a slot from DOM resets its slottable's slot assignment.
 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.
+PASS throw TypeError if the passed values are neither Element nor Text
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api.html b/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/imperative-slot-api.html
@@ -285,4 +285,19 @@
   assert_equals(tTree.c2.assignedSlot, tTree.s1);
 }, 'Removing a node from the document does not break manually assigned slot linkage.');
 
+test(() => {
+  const inputValues = [
+    ['Attr', document.createAttribute('bar')],
+    ['Comment', document.createComment('bar')],
+    ['DocumentFragment', document.createDocumentFragment()],
+    ['DocumentType', document.implementation.createDocumentType('html', '', '')]
+  ];
+  for (const [label, input] of inputValues) {
+    assert_throws_js(TypeError, () => {
+      const slot = document.createElement('slot');
+      slot.assign(input);
+    }, label);
+  }
+}, 'throw TypeError if the passed values are neither Element nor Text');
+
 </script>
diff --git a/Source/WebCore/html/HTMLSlotElement.cpp b/Source/WebCore/html/HTMLSlotElement.cpp
@@ -36,6 +36,7 @@
 #include "Text.h"
 #include <wtf/IsoMallocInlines.h>
 #include <wtf/SetForScope.h>
+#include <wtf/StdLibExtras.h>
 
 namespace WebCore {
 
@@ -165,7 +166,7 @@ Vector<Ref<Element>> HTMLSlotElement::assignedElements(const AssignedNodesOption
     });
 }
 
-void HTMLSlotElement::assign(FixedVector<std::reference_wrapper<Node>>&& nodes)
+void HTMLSlotElement::assign(FixedVector<ElementOrText>&& nodes)
 {
     RefPtr shadowRoot = containingShadowRoot();
     RefPtr host = shadowRoot ? shadowRoot->host() : nullptr;
@@ -175,12 +176,19 @@ void HTMLSlotElement::assign(FixedVector<std::reference_wrapper<Node>>&& nodes)
     }
 
     auto previous = std::exchange(m_manuallyAssignedNodes, { });
-    HashSet<Ref<Node>> seenNodes;
-    m_manuallyAssignedNodes = WTF::compactMap(nodes, [&](Node& node) -> std::optional<WeakPtr<Node>> {
-        if (seenNodes.contains(node))
-            return std::nullopt;
-        seenNodes.add(node);
-        return WeakPtr { node };
+    HashSet<RefPtr<Node>> seenNodes;
+    m_manuallyAssignedNodes = WTF::compactMap(nodes, [&seenNodes](ElementOrText& node) -> std::optional<WeakPtr<Node>> {
+        auto mapper = [&seenNodes]<typename T>(RefPtr<T>& node) -> std::optional<WeakPtr<Node>> {
+            if (seenNodes.contains(node))
+                return std::nullopt;
+            seenNodes.add(node);
+            return WeakPtr { node };
+        };
+
+        return WTF::switchOn(node,
+            [&mapper](RefPtr<Element>& node) { return mapper(node); },
+            [&mapper](RefPtr<Text>& node) { return mapper(node); }
+        );
     });
 
     if (RefPtr shadowRoot = containingShadowRoot(); shadowRoot && shadowRoot->slotAssignmentMode() == SlotAssignmentMode::Manual)

diff --git a/Source/WebCore/html/HTMLSlotElement.h b/Source/WebCore/html/HTMLSlotElement.h
@@ -32,6 +32,8 @@ namespace WebCore {
 class HTMLSlotElement final : public HTMLElement {
     WTF_MAKE_ISO_ALLOCATED(HTMLSlotElement);
 public:
+    using ElementOrText = std::variant<RefPtr<Element>, RefPtr<Text>>;
+
     static Ref<HTMLSlotElement> create(const QualifiedName&, Document&);
 
     const Vector<WeakPtr<Node>>* assignedNodes() const;
@@ -41,7 +43,7 @@ class HTMLSlotElement final : public HTMLElement {
     Vector<Ref<Node>> assignedNodes(const AssignedNodesOptions&) const;
     Vector<Ref<Element>> assignedElements(const AssignedNodesOptions&) const;
 
-    void assign(FixedVector<std::reference_wrapper<Node>>&&);
+    void assign(FixedVector<ElementOrText>&&);
     const Vector<WeakPtr<Node>>& manuallyAssignedNodes() const { return m_manuallyAssignedNodes; }
     void removeManuallyAssignedNode(Node&);
 

diff --git a/Source/WebCore/html/HTMLSlotElement.idl b/Source/WebCore/html/HTMLSlotElement.idl
@@ -30,7 +30,7 @@
     [CEReactions=NotNeeded, Reflect] attribute DOMString name;
     sequence<Node> assignedNodes(optional AssignedNodesOptions options);
     sequence<Element> assignedElements(optional AssignedNodesOptions options);
-    [EnabledBySetting=ImperativeSlotAPIEnabled] undefined assign(Node... nodes); // FIXME: This should be (Element or Text) instead of Node.
+    [EnabledBySetting=ImperativeSlotAPIEnabled] undefined assign((Element or Text)... nodes);
 };
 
 dictionary AssignedNodesOptions {