Don't run disconnected callback on already disconnected custom elements (#35883)

* Add missing step 11 of remove a node algorithm

Signed-off-by: Xiaocheng Hu <xiaochengh.work@gmail.com>

* Rebaseline tests

Signed-off-by: Xiaocheng Hu <xiaochengh.work@gmail.com>

---------

Signed-off-by: Xiaocheng Hu <xiaochengh.work@gmail.com>

1 files changed, 38 insertions, 19 deletions

diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
index d248028cc2b..ce0481c372c 100644
--- a/components/script/dom/node.rs
+++ b/components/script/dom/node.rs
@@ -316,7 +316,8 @@ impl Node {
         self.layout_data.borrow_mut().take();
     }
 
-    /// Clean up flags and unbind from tree.
+    /// Clean up flags and runs steps 11-14 of remove a node.
+    /// <https://dom.spec.whatwg.org/#concept-node-remove>
     pub(crate) fn complete_remove_subtree(root: &Node, context: &UnbindContext) {
         // Flags that reset when a node is disconnected
         const RESET_FLAGS: NodeFlags = NodeFlags::IS_IN_A_DOCUMENT_TREE
@@ -340,20 +341,27 @@ impl Node {
             }
         }
 
+        // Step 12.
+        let is_parent_connected = context.parent.is_connected();
+
         for node in root.traverse_preorder(ShadowIncluding::Yes) {
             node.clean_up_style_and_layout_data();
 
+            // Step 11 & 14.1. Run the removing steps.
             // This needs to be in its own loop, because unbind_from_tree may
             // rely on the state of IS_IN_DOC of the context node's descendants,
             // e.g. when removing a <form>.
             vtable_for(&node).unbind_from_tree(context);
-            // https://dom.spec.whatwg.org/#concept-node-remove step 14
-            if let Some(element) = node.as_custom_element() {
-                ScriptThread::enqueue_callback_reaction(
-                    &element,
-                    CallbackReaction::Disconnected,
-                    None,
-                );
+
+            // Step 12 & 14.2. Enqueue disconnected custom element reactions.
+            if is_parent_connected {
+                if let Some(element) = node.as_custom_element() {
+                    ScriptThread::enqueue_callback_reaction(
+                        &element,
+                        CallbackReaction::Disconnected,
+                        None,
+                    );
+                }
             }
         }
     }
@@ -2461,10 +2469,15 @@ impl Node {
     /// <https://dom.spec.whatwg.org/#concept-node-remove>
     fn remove(node: &Node, parent: &Node, suppress_observers: SuppressObserver) {
         parent.owner_doc().add_script_and_layout_blocker();
+
+        // Step 2.
         assert!(
             node.GetParentNode()
                 .is_some_and(|node_parent| &*node_parent == parent)
         );
+
+        // Step 3. Run the live range pre-remove steps.
+        // https://dom.spec.whatwg.org/#live-range-pre-remove-steps
         let cached_index = {
             if parent.ranges_is_empty() {
                 None
@@ -2480,20 +2493,25 @@ impl Node {
                 Some(index)
             }
         };
-        // Step 6. pre-removing steps for node iterators
-        // Step 7.
+
+        // TODO: Step 4. Pre-removing steps for node iterators
+
+        // Step 5.
         let old_previous_sibling = node.GetPreviousSibling();
-        // Step 8.
+
+        // Step 6.
         let old_next_sibling = node.GetNextSibling();
-        // Steps 9-10 are handled in unbind_from_tree.
+
+        // Step 7. Remove node from its parent's children.
+        // Step 11-14. Run removing steps and enqueue disconnected custom element reactions for the subtree.
         parent.remove_child(node, cached_index);
 
-        // Step 12. If node is assigned, then run assign slottables for node’s assigned slot.
+        // Step 8. If node is assigned, then run assign slottables for node’s assigned slot.
         if let Some(slot) = node.assigned_slot() {
             slot.assign_slottables();
         }
 
-        // Step 13. If parent’s root is a shadow root, and parent is a slot whose assigned nodes is the empty list,
+        // Step 9. If 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.
         if parent.is_in_a_shadow_tree() {
             if let Some(slot_element) = parent.downcast::<HTMLSlotElement>() {
@@ -2503,22 +2521,23 @@ impl Node {
             }
         }
 
-        // Step 14. If node has an inclusive descendant that is a slot:
+        // Step 10. If node has an inclusive descendant that is a slot:
         let has_slot_descendant = node
             .traverse_preorder(ShadowIncluding::No)
             .any(|elem| elem.is::<HTMLSlotElement>());
         if has_slot_descendant {
-            // Step 14.1 Run assign slottables for a tree with parent’s root.
+            // Step 10.1 Run assign slottables for a tree with parent’s root.
             parent
                 .GetRootNode(&GetRootNodeOptions::empty())
                 .assign_slottables_for_a_tree();
 
-            // Step 14.2 Run assign slottables for a tree with node.
+            // Step 10.2 Run assign slottables for a tree with node.
             node.assign_slottables_for_a_tree();
         }
 
-        // Step 11. transient registered observers
-        // Step 12.
+        // TODO: Step 15. transient registered observers
+
+        // Step 16.
         if let SuppressObserver::Unsuppressed = suppress_observers {
             vtable_for(parent).children_changed(&ChildrenMutation::replace(
                 old_previous_sibling.as_deref(),