Attachment 348364 Details for Bug 169718 – Fixes the bug

[patch] Fixes the bug

bug-169718-20180828172040.patch (text/plain), 9.54 KB, created by Ryosuke Niwa on 2018-08-28 17:20:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-28 17:20:41 PDT

Size: 9.54 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 235438)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,38 @@
>+2018-08-28  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Changes to slot children should trigger slotchange
>+        https://bugs.webkit.org/show_bug.cgi?id=169718
>+        <rdar://problem/43317496>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix the bug that slotchange event is not fired when a slot's fallback content is updated now that slotchange event
>+        is more formally specified.
>+
>+        This particular behavior corresponds to step 7.5. of the concept *to insert a node* where it says:
>+        "[I]f parentâs root is a shadow root, and parent is a slot whose assigned nodes is the empty list, then run signal
>+        a slot change for parent."
>+
>+        See https://dom.spec.whatwg.org/#concept-node-insert
>+
>+        Test: imported/w3c/web-platform-tests/shadow-dom/slotchange.html 
>+
>+        * dom/Element.cpp:
>+        (WebCore::Element::childrenChanged): Updated the comment.
>+        * dom/ShadowRoot.cpp:
>+        (WebCore::ShadowRoot::addSlotElementByName): Added an assertion.
>+        (WebCore::ShadowRoot::slotFallbackDidChange): Added.
>+        * dom/ShadowRoot.h:
>+        * dom/SlotAssignment.cpp:
>+        (WebCore::SlotAssignment::slotFallbackDidChange): Added. When the assigned nodes is empty, we enqueue a slotchange.
>+        Because assignedNodesForSlot invokes assignSlots, this can be O(n) but we don't expect mutating slot's fallback
>+        contents and shadow host's children in turn to be a common scenario so this shouldn't be an issue in practice.
>+        * dom/SlotAssignment.h:
>+        * html/HTMLSlotElement.cpp:
>+        (WebCore::HTMLSlotElement::insertedIntoAncestor): Be explicit about auto* being used here.
>+        (WebCore::HTMLSlotElement::childrenChanged): Added. Invokes slotFallbackDidChange whenver child node is muated.
>+        * html/HTMLSlotElement.h:
>+
> 2018-08-28  Youenn Fablet  <youenn@apple.com>
> 
>         MediaDevices should be collectable as soon as its document is stopped
>Index: Source/WebCore/dom/Element.cpp
>===================================================================
>--- Source/WebCore/dom/Element.cpp	(revision 235438)
>+++ Source/WebCore/dom/Element.cpp	(working copy)
>@@ -2178,7 +2178,7 @@ void Element::childrenChanged(const Chil
>         switch (change.type) {
>         case ElementInserted:
>         case ElementRemoved:
>-            // For elements, we notify shadowRoot in Element::insertedInto and Element::removedFrom.
>+            // For elements, we notify shadowRoot in Element::insertedIntoAncestor and Element::removedFromAncestor.
>             break;
>         case AllChildrenRemoved:
>         case AllChildrenReplaced:
>Index: Source/WebCore/dom/ShadowRoot.cpp
>===================================================================
>--- Source/WebCore/dom/ShadowRoot.cpp	(revision 235438)
>+++ Source/WebCore/dom/ShadowRoot.cpp	(working copy)
>@@ -183,6 +183,7 @@ HTMLSlotElement* ShadowRoot::findAssigne
> 
> void ShadowRoot::addSlotElementByName(const AtomicString& name, HTMLSlotElement& slot)
> {
>+    ASSERT(&slot.rootNode() == this);
>     if (!m_slotAssignment)
>         m_slotAssignment = std::make_unique<SlotAssignment>();
> 
>@@ -194,6 +195,12 @@ void ShadowRoot::removeSlotElementByName
>     return m_slotAssignment->removeSlotElementByName(name, slot, *this);
> }
> 
>+void ShadowRoot::slotFallbackDidChange(HTMLSlotElement& slot)
>+{
>+    ASSERT(&slot.rootNode() == this);
>+    return m_slotAssignment->slotFallbackDidChange(slot, *this);
>+}
>+
> const Vector<Node*>* ShadowRoot::assignedNodesForSlot(const HTMLSlotElement& slot)
> {
>     if (!m_slotAssignment)
>Index: Source/WebCore/dom/ShadowRoot.h
>===================================================================
>--- Source/WebCore/dom/ShadowRoot.h	(revision 235438)
>+++ Source/WebCore/dom/ShadowRoot.h	(working copy)
>@@ -74,6 +74,7 @@ public:
> 
>     void addSlotElementByName(const AtomicString&, HTMLSlotElement&);
>     void removeSlotElementByName(const AtomicString&, HTMLSlotElement&);
>+    void slotFallbackDidChange(HTMLSlotElement&);
> 
>     void didRemoveAllChildrenOfShadowHost();
>     void didChangeDefaultSlot();
>@@ -107,7 +108,6 @@ private:
>     Element* m_host { nullptr };
> 
>     std::unique_ptr<Style::Scope> m_styleScope;
>-
>     std::unique_ptr<SlotAssignment> m_slotAssignment;
> };
> 
>Index: Source/WebCore/dom/SlotAssignment.cpp
>===================================================================
>--- Source/WebCore/dom/SlotAssignment.cpp	(revision 235438)
>+++ Source/WebCore/dom/SlotAssignment.cpp	(working copy)
>@@ -123,6 +123,16 @@ void SlotAssignment::removeSlotElementBy
>     ASSERT(slotInfo.element || m_needsToResolveSlotElements);
> }
> 
>+void SlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot)
>+{
>+    if (shadowRoot.mode() == ShadowRootMode::UserAgent)
>+        return;
>+
>+    bool usesFallbackContent = !assignedNodesForSlot(slotElement, shadowRoot);
>+    if (!usesFallbackContent)
>+        slotElement.enqueueSlotChangeEvent();
>+}
>+
> void SlotAssignment::didChangeSlot(const AtomicString& slotAttrValue, ShadowRoot& shadowRoot)
> {
>     auto& slotName = slotNameFromAttributeValue(slotAttrValue);
>Index: Source/WebCore/dom/SlotAssignment.h
>===================================================================
>--- Source/WebCore/dom/SlotAssignment.h	(revision 235438)
>+++ Source/WebCore/dom/SlotAssignment.h	(working copy)
>@@ -51,6 +51,7 @@ public:
> 
>     void addSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
>     void removeSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&);
>+    void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&);
> 
>     void didChangeSlot(const AtomicString&, ShadowRoot&);
>     void enqueueSlotChangeEvent(const AtomicString&, ShadowRoot&);
>Index: Source/WebCore/html/HTMLSlotElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLSlotElement.cpp	(revision 235438)
>+++ Source/WebCore/html/HTMLSlotElement.cpp	(working copy)
>@@ -57,7 +57,7 @@ HTMLSlotElement::InsertedIntoAncestorRes
>     ASSERT_UNUSED(insertionResult, insertionResult == InsertedIntoAncestorResult::Done);
> 
>     if (insertionType.treeScopeChanged && isInShadowTree()) {
>-        if (auto shadowRoot = containingShadowRoot())
>+        if (auto* shadowRoot = containingShadowRoot())
>             shadowRoot->addSlotElementByName(attributeWithoutSynchronization(nameAttr), *this);
>     }
> 
>@@ -75,6 +75,16 @@ void HTMLSlotElement::removedFromAncesto
>     HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
> }
> 
>+void HTMLSlotElement::childrenChanged(const ChildChange& childChange)
>+{
>+    HTMLElement::childrenChanged(childChange);
>+
>+    if (isInShadowTree()) {
>+        if (auto* shadowRoot = containingShadowRoot())
>+            shadowRoot->slotFallbackDidChange(*this);
>+    }
>+}
>+
> void HTMLSlotElement::attributeChanged(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue, AttributeModificationReason reason)
> {
>     HTMLElement::attributeChanged(name, oldValue, newValue, reason);
>Index: Source/WebCore/html/HTMLSlotElement.h
>===================================================================
>--- Source/WebCore/html/HTMLSlotElement.h	(revision 235438)
>+++ Source/WebCore/html/HTMLSlotElement.h	(working copy)
>@@ -51,6 +51,7 @@ private:
> 
>     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
>     void removedFromAncestor(RemovalType, ContainerNode&) final;
>+    void childrenChanged(const ChildChange&) final;
>     void attributeChanged(const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue, AttributeModificationReason) final;
> 
>     bool m_inSignalSlotList { false };
>Index: LayoutTests/imported/w3c/ChangeLog
>===================================================================
>--- LayoutTests/imported/w3c/ChangeLog	(revision 235448)
>+++ LayoutTests/imported/w3c/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2018-08-28  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Changes to slot children should trigger slotchange
>+        https://bugs.webkit.org/show_bug.cgi?id=169718
>+        <rdar://problem/43317496>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rebaselined the test now that relevant test cases pass.
>+
>+        * web-platform-tests/shadow-dom/slotchange-expected.txt:
>+
> 2018-08-28  Aditya Keerthi  <akeerthi@apple.com>
> 
>         [iOS] Support inputmode=none
>Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt
>===================================================================
>--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt	(revision 235438)
>+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt	(working copy)
>@@ -10,8 +10,8 @@ PASS slotchange event: Change slot= attr
> TIMEOUT slotchange event: Change slot's name= attribute so that a node is assigned to the slot. Test timed out
> PASS slotchange event: Add a fallback content. 
> PASS slotchange event: Remove a fallback content. 
>-TIMEOUT slotchange event: Add a fallback content to nested slots. Test timed out
>-TIMEOUT slotchange event: Remove a fallback content from nested slots. Test timed out
>+PASS slotchange event: Add a fallback content to nested slots. 
>+PASS slotchange event: Remove a fallback content from nested slots. 
> TIMEOUT slotchange event: Insert a slot before an existing slot. Test timed out
> TIMEOUT slotchange event: Remove a preceding slot. Test timed out
> PASS slotchange event: A slot is assigned to another slot.

Actions: View | Formatted Diff | Diff

Attachments on bug 169718: 348058 | 348262 | 348266 | 348276 | 348364 | 348374 | 348382 | 348390