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>
This commit is contained in:
Xiaocheng Hu 2025-03-13 19:07:08 +08:00 committed by GitHub
parent 2ccb818a60
commit 3b724d5bbf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 38 additions and 52 deletions

View file

@ -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 nodes assigned slot.
// Step 8. If node is assigned, then run assign slottables for nodes assigned slot.
if let Some(slot) = node.assigned_slot() {
slot.assign_slottables();
}
// Step 13. If parents root is a shadow root, and parent is a slot whose assigned nodes is the empty list,
// Step 9. If parents 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 parents root.
// Step 10.1 Run assign slottables for a tree with parents 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(),