WebKit Bugzilla
Attachment 348477 Details for
Bug 189144
: slotchange event doesn't get fired when inserting, removing, or renaming slot elements
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
WIP
wip189144.patch (text/plain), 22.74 KB, created by
Ryosuke Niwa
on 2018-08-29 20:47:45 PDT
(
hide
)
Description:
WIP
Filename:
MIME Type:
Creator:
Ryosuke Niwa
Created:
2018-08-29 20:47:45 PDT
Size:
22.74 KB
patch
obsolete
>Index: Source/WebCore/dom/ContainerNode.cpp >=================================================================== >--- Source/WebCore/dom/ContainerNode.cpp (revision 235483) >+++ Source/WebCore/dom/ContainerNode.cpp (working copy) >@@ -59,6 +59,7 @@ > #include "SVGUseElement.h" > #include "ScriptDisallowedScope.h" > #include "SelectorQuery.h" >+#include "SlotAssignment.h" > #include "TemplateContentDocumentFragment.h" > #include <algorithm> > #include <wtf/IsoMallocInlines.h> >@@ -104,6 +105,9 @@ > WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates; > ScriptDisallowedScope::InMainThread scriptDisallowedScope; > >+ if (UNLIKELY(isShadowRoot() || isInShadowTree())) >+ containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval(); >+ > document().nodeChildrenWillBeRemoved(*this); > > while (RefPtr<Node> child = m_firstChild) { >@@ -150,6 +154,9 @@ > WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates; > ScriptDisallowedScope::InMainThread scriptDisallowedScope; > >+ if (UNLIKELY(isShadowRoot() || isInShadowTree())) >+ containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval(); >+ > document().nodeWillBeRemoved(childToRemove); > > ASSERT_WITH_SECURITY_IMPLICATION(childToRemove.parentNode() == this); >@@ -181,6 +188,10 @@ > NodeVector postInsertionNotificationTargets; > { > ScriptDisallowedScope::InMainThread scriptDisallowedScope; >+ >+ if (UNLIKELY(containerNode.isShadowRoot() || containerNode.isInShadowTree())) >+ containerNode.containingShadowRoot()->resolveSlotsBeforeNodeInsertionOrRemoval(); >+ > doNodeInsertion(); > ChildListMutationScope(containerNode).childAdded(child); > postInsertionNotificationTargets = notifyChildNodeInserted(containerNode, child); >Index: Source/WebCore/dom/ShadowRoot.cpp >=================================================================== >--- Source/WebCore/dom/ShadowRoot.cpp (revision 235483) >+++ Source/WebCore/dom/ShadowRoot.cpp (working copy) >@@ -82,6 +82,9 @@ > // to access it Document reference after that. > willBeDeletedFrom(document()); > >+ ASSERT(!m_hasBegunDeletingDetachedChildren); >+ m_hasBegunDeletingDetachedChildren = true; >+ > // We must remove all of our children first before the TreeScope destructor > // runs so we don't go through Node::setTreeScopeRecursively for each child with a > // destructed tree scope in each descendant. >Index: Source/WebCore/dom/ShadowRoot.h >=================================================================== >--- Source/WebCore/dom/ShadowRoot.h (revision 235483) >+++ Source/WebCore/dom/ShadowRoot.h (working copy) >@@ -58,6 +58,10 @@ > bool resetStyleInheritance() const { return m_resetStyleInheritance; } > void setResetStyleInheritance(bool); > >+ bool hasBegunDeletingDetachedChildren() const { return m_hasBegunDeletingDetachedChildren; } >+ >+ ShadowRootMode mode() const { return m_type; } >+ > Element* host() const { return m_host; } > void setHost(Element* host) { m_host = host; } > >@@ -66,8 +70,6 @@ > > Element* activeElement() const; > >- ShadowRootMode mode() const { return m_type; } >- > void removeAllEventListeners() override; > > HTMLSlotElement* findAssignedSlot(const Node&); >@@ -75,6 +77,7 @@ > void addSlotElementByName(const AtomicString&, HTMLSlotElement&); > void removeSlotElementByName(const AtomicString&, HTMLSlotElement&); > void slotFallbackDidChange(HTMLSlotElement&); >+ void resolveSlotsBeforeNodeInsertionOrRemoval(); > > void didRemoveAllChildrenOfShadowHost(); > void didChangeDefaultSlot(); >@@ -103,6 +106,7 @@ > void removedFromAncestor(RemovalType, ContainerNode& insertionPoint) override; > > bool m_resetStyleInheritance { false }; >+ bool m_hasBegunDeletingDetachedChildren { false }; > ShadowRootMode m_type { ShadowRootMode::UserAgent }; > > Element* m_host { nullptr }; >Index: Source/WebCore/dom/SlotAssignment.cpp >=================================================================== >--- Source/WebCore/dom/SlotAssignment.cpp (revision 235483) >+++ Source/WebCore/dom/SlotAssignment.cpp (working copy) >@@ -85,13 +85,37 @@ > }); > > auto& slot = *addResult.iterator->value; >- if (!slot.hasSlotElements()) >+ if (!slot.hasSlotElements()) { > slot.element = makeWeakPtr(slotElement); >- else { >- slot.element = nullptr; >-#ifndef NDEBUG >- m_needsToResolveSlotElements = true; >-#endif >+ if (shadowRoot.mode() != ShadowRootMode::UserAgent) { >+ if (!m_slotAssignmentsIsValid) >+ assignSlots(shadowRoot); >+ if (!slot.assignedNodes.isEmpty()) >+ slotElement.enqueueSlotChangeEvent(); >+ } >+ } else { >+ ASSERT(slot.element); // resolveSlotsBeforeNodeInsertionOrRemoval must have been called. >+ bool newSlotIsFirstSlot = false; >+ if (shadowRoot.mode() != ShadowRootMode::UserAgent) { >+ auto& previouslyFirstSlot = *slot.element; >+ if (!m_slotAssignmentsIsValid) >+ assignSlots(shadowRoot); >+ if (!slot.assignedNodes.isEmpty()) { >+ HTMLSlotElement* slotBeforePrevioslyFirstSlot = Traversal<HTMLSlotElement>::previous(previouslyFirstSlot); >+ if (slotBeforePrevioslyFirstSlot == &slotElement) { >+ slotElement.enqueueSlotChangeEvent(); >+ slot.element->enqueueSlotChangeEvent(); >+ newSlotIsFirstSlot = slot.elementCount == 2; >+ } >+ } >+ } >+ if (newSlotIsFirstSlot) >+ slot.element = makeWeakPtr(slotElement); >+ else { >+ slot.element = nullptr; >+ m_needsToResolveSlotElements = true; >+ } >+ > } > slot.elementCount++; > } >@@ -109,14 +133,29 @@ > auto* slot = m_slots.get(slotNameFromAttributeValue(name)); > RELEASE_ASSERT(slot && slot->hasSlotElements()); > >+ bool removedFirstSlot = slot->element == &slotElement; > slot->elementCount--; >- if (slot->element == &slotElement) { >+ if (removedFirstSlot) { > slot->element = nullptr; >-#ifndef NDEBUG > m_needsToResolveSlotElements = true; >-#endif > } > ASSERT(slot->element || m_needsToResolveSlotElements); >+ >+ if (shadowRoot.mode() == ShadowRootMode::UserAgent || !removedFirstSlot || shadowRoot.hasBegunDeletingDetachedChildren()) >+ return; >+ >+ if (!m_slotAssignmentsIsValid) >+ assignSlots(shadowRoot); >+ >+ if (slot->assignedNodes.isEmpty()) >+ return; >+ >+ slotElement.enqueueSlotChangeEvent(); >+ if (slot->elementCount > 1) { >+ resolveAllSlotElements(shadowRoot); >+ ASSERT(slot->element); >+ slot->element->enqueueSlotChangeEvent(); >+ } > } > > void SlotAssignment::slotFallbackDidChange(HTMLSlotElement& slotElement, ShadowRoot& shadowRoot) >@@ -129,6 +168,13 @@ > slotElement.enqueueSlotChangeEvent(); > } > >+void SlotAssignment::resolveSlotsBeforeNodeInsertionOrRemoval(ShadowRoot& shadowRoot) >+{ >+ ASSERT(shadowRoot.mode() != ShadowRootMode::UserAgent); >+ if (m_needsToResolveSlotElements) >+ resolveAllSlotElements(shadowRoot); >+} >+ > void SlotAssignment::didChangeSlot(const AtomicString& slotAttrValue, ShadowRoot& shadowRoot) > { > auto& slotName = slotNameFromAttributeValue(slotAttrValue); >@@ -196,10 +242,8 @@ > > void SlotAssignment::resolveAllSlotElements(ShadowRoot& shadowRoot) > { >-#ifndef NDEBUG > ASSERT(m_needsToResolveSlotElements); > m_needsToResolveSlotElements = false; >-#endif > > // FIXME: It's inefficient to reset all values. We should be able to void this in common case. > for (auto& entry : m_slots) >Index: Source/WebCore/dom/SlotAssignment.h >=================================================================== >--- Source/WebCore/dom/SlotAssignment.h (revision 235483) >+++ Source/WebCore/dom/SlotAssignment.h (working copy) >@@ -53,6 +53,7 @@ > void addSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&); > void removeSlotElementByName(const AtomicString&, HTMLSlotElement&, ShadowRoot&); > void slotFallbackDidChange(HTMLSlotElement&, ShadowRoot&); >+ void resolveSlotsBeforeNodeInsertionOrRemoval(ShadowRoot&); > > void didChangeSlot(const AtomicString&, ShadowRoot&); > void enqueueSlotChangeEvent(const AtomicString&, ShadowRoot&); >@@ -88,12 +89,18 @@ > > #ifndef NDEBUG > HashSet<HTMLSlotElement*> m_slotElementsForConsistencyCheck; >- bool m_needsToResolveSlotElements { false }; > #endif > >+ bool m_needsToResolveSlotElements { false }; > bool m_slotAssignmentsIsValid { false }; > }; > >+inline void ShadowRoot::resolveSlotsBeforeNodeInsertionOrRemoval() >+{ >+ if (mode() != ShadowRootMode::UserAgent && m_slotAssignment) >+ m_slotAssignment->resolveSlotsBeforeNodeInsertionOrRemoval(*this); >+} >+ > inline void ShadowRoot::didRemoveAllChildrenOfShadowHost() > { > if (m_slotAssignment) // FIXME: This is incorrect when there were no elements or text nodes removed. >Index: LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html >=================================================================== >--- LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html (revision 235483) >+++ LayoutTests/fast/shadow-dom/slotchange-event-bubbling.html (working copy) >@@ -75,7 +75,7 @@ > test_slotchange_event_bubbles('open', true); > > function test_single_slotchange_event_for_nested_slots(outerMode, innerMode, connected) { >- promise_test(() => { >+ promise_test(async () => { > const outerHost = document.createElement('outer-host'); > if (connected) > document.body.appendChild(outerHost); >@@ -91,6 +91,8 @@ > const innerSlotParent = innerShadow.querySelector('div'); > const innerSlot = innerShadow.querySelector('slot'); > >+ await Promise.resolve(); >+ > const observer = create_slotchange_observer(); > observer.observe(outerSlot); > observer.observe(innerHost); >@@ -109,23 +111,23 @@ > outerHost.textContent = ' '; > > assert_array_equals(observer.takeLog(), [], 'slotchange event must not be fired synchronously'); >- return Promise.resolve().then(() => { >- const log = observer.takeLog(); >+ await Promise.resolve(); > >- const events = new Set(log.map((entry) => entry.event)); >- assert_equals(events.size, 1, 'Mutating the assigned content of a slot must fire exactly one slotchange event'); >+ const log = observer.takeLog(); > >- assert_slotchange_log(log[0], outerSlot, outerSlot, 'slotchange event must be dispatched at the slot element first'); >- assert_slotchange_log(log[1], innerSlot, outerSlot, 'slotchange event must bubble up from a slot element to its assigned slot'); >- assert_slotchange_log(log[2], innerSlotParent, outerSlot, 'slotchange event must bubble up to the parent node of a slot'); >- assert_slotchange_log(log[3], innerShadow, outerSlot, 'slotchange event must bubble up to the shadow root'); >- assert_slotchange_log(log[4], innerHost, outerSlot, >- 'slotchange event must bubble up to the shadow host if the host is a descendent of the tree in which the event was fired'); >- assert_slotchange_log(log[5], outerHostParent, outerSlot, >- 'slotchange event must bubble up to the parent of an inner shadow host'); >- assert_slotchange_log(log[6], outerShadow, outerSlot, 'slotchange event must bubble up to the shadow root'); >- assert_equals(log.length, 7, 'slotchange must not bubble beyond the shadow root in which the event was fired'); >- }); >+ const events = new Set(log.map((entry) => entry.event)); >+ assert_equals(events.size, 1, 'Mutating the assigned content of a slot must fire exactly one slotchange event'); >+ >+ assert_slotchange_log(log[0], outerSlot, outerSlot, 'slotchange event must be dispatched at the slot element first'); >+ assert_slotchange_log(log[1], innerSlot, outerSlot, 'slotchange event must bubble up from a slot element to its assigned slot'); >+ assert_slotchange_log(log[2], innerSlotParent, outerSlot, 'slotchange event must bubble up to the parent node of a slot'); >+ assert_slotchange_log(log[3], innerShadow, outerSlot, 'slotchange event must bubble up to the shadow root'); >+ assert_slotchange_log(log[4], innerHost, outerSlot, >+ 'slotchange event must bubble up to the shadow host if the host is a descendent of the tree in which the event was fired'); >+ assert_slotchange_log(log[5], outerHostParent, outerSlot, >+ 'slotchange event must bubble up to the parent of an inner shadow host'); >+ assert_slotchange_log(log[6], outerShadow, outerSlot, 'slotchange event must bubble up to the shadow root'); >+ assert_equals(log.length, 7, 'slotchange must not bubble beyond the shadow root in which the event was fired'); > }, `A single slotchange event must bubble from a ${connected ? 'connected' : 'disconnected'} ${innerMode}-mode shadow tree to` > + `a slot in its parent ${outerMode}-mode shadow tree`); > } >Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt >=================================================================== >--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt (revision 235483) >+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-customelements-expected.txt (working copy) >@@ -1,3 +1,3 @@ > >-FAIL slotchange must fire on initialization of custom elements with slotted children assert_true: expected true got false >+PASS slotchange must fire on initialization of custom elements with slotted children > >Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt >=================================================================== >--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt (revision 235483) >+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-event-expected.txt (working copy) >@@ -1,10 +1,4 @@ >-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0 >-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0 >-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0 >-CONSOLE MESSAGE: line 2659: Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0 > >-Harness Error (FAIL), message = Error: assert_equals: slotchange must not be fired on a slot element if the assigned nodes changed after the slot was removed expected 2 but got 0 >- > PASS slotchange event must fire on a default slot element inside an open shadow root in a document > PASS slotchange event must fire on a default slot element inside a closed shadow root in a document > PASS slotchange event must fire on a default slot element inside an open shadow root not in a document >@@ -17,10 +11,10 @@ > PASS slotchange event must not fire on a slot element inside a closed shadow root in a document when another slot's assigned nodes change > PASS slotchange event must not fire on a slot element inside an open shadow root not in a document when another slot's assigned nodes change > PASS slotchange event must not fire on a slot element inside a closed shadow root not in a document when another slot's assigned nodes change >-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0 >-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0 >-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root not in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0 >-FAIL slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root not in a document assert_equals: slotchange must be fired on a slot element if there is assigned nodes when the slot was inserted expected 1 but got 0 >+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root in a document >+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root in a document >+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside an open shadow root not in a document >+PASS slotchange event must fire on a slot element when a shadow host has a slotable and the slot was inserted and must not fire when the shadow host was mutated after the slot was removed inside a closed shadow root not in a document > PASS slotchange event must fire on a slot element inside an open shadow root in a document even if the slot was removed immediately after the assigned nodes were mutated > PASS slotchange event must fire on a slot element inside a closed shadow root in a document even if the slot was removed immediately after the assigned nodes were mutated > PASS slotchange event must fire on a slot element inside an open shadow root not in a document even if the slot was removed immediately after the assigned nodes were mutated >@@ -29,10 +23,10 @@ > PASS slotchange event must fire on a slot element inside a closed shadow root in a document when innerHTML modifies the children of the shadow host > PASS slotchange event must fire on a slot element inside an open shadow root not in a document when innerHTML modifies the children of the shadow host > PASS slotchange event must fire on a slot element inside a closed shadow root not in a document when innerHTML modifies the children of the shadow host >-FAIL slotchange event must fire on a slot element inside an open shadow root in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot> >-FAIL slotchange event must fire on a slot element inside a closed shadow root in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot> >-FAIL slotchange event must fire on a slot element inside an open shadow root not in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot> >-FAIL slotchange event must fire on a slot element inside a closed shadow root not in a document when nested slots's contents change assert_equals: slotchange event's target must be the inner slot element at 1st slotchange expected Element node <slot></slot> but got Element node <slot></slot> >+PASS slotchange event must fire on a slot element inside an open shadow root in a document when nested slots's contents change >+PASS slotchange event must fire on a slot element inside a closed shadow root in a document when nested slots's contents change >+PASS slotchange event must fire on a slot element inside an open shadow root not in a document when nested slots's contents change >+PASS slotchange event must fire on a slot element inside a closed shadow root not in a document when nested slots's contents change > PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside an open shadow root in a document when slots's contents change > PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside a closed shadow root in a document when slots's contents change > PASS slotchange event must fire at the end of current microtask after mutation observers are invoked inside an open shadow root not in a document when slots's contents change >Index: LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt >=================================================================== >--- LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt (revision 235483) >+++ LayoutTests/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt (working copy) >@@ -1,19 +1,17 @@ > >-Harness Error (TIMEOUT), message = null >- > PASS slotchange event: Append a child to a host. > PASS slotchange event: Remove a child from a host. > PASS slotchange event: Remove a child before adding an event listener. > PASS slotchange event: Change slot= attribute to make it un-assigned. >-TIMEOUT slotchange event: Change slot's name= attribute so that none is assigned. Test timed out >+PASS slotchange event: Change slot's name= attribute so that none is assigned. > PASS slotchange event: Change slot= attribute to make it assigned. >-TIMEOUT slotchange event: Change slot's name= attribute so that a node is assigned to the slot. Test timed out >+PASS slotchange event: Change slot's name= attribute so that a node is assigned to the slot. > PASS slotchange event: Add a fallback content. > PASS slotchange event: Remove a fallback content. > 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: Insert a slot before an existing slot. >+PASS slotchange event: Remove a preceding slot. > PASS slotchange event: A slot is assigned to another slot. > PASS slotchange event: Even if distributed nodes do not change, slotchange should be fired if assigned nodes are changed. >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
ews-watchlist
:
commit-queue-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189144
:
348477
|
348486
|
348490
|
348583
|
348594
|
348697
|
348699
|
348709
|
348710
|
348711
|
348839