From 1ab55e6b113b7bea1abb27cc75c52f60f40306b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Sat, 4 Jan 2025 00:27:43 +0100 Subject: [PATCH] Fix IS_IN_SHADOW_TREE flag for descendants after Node::remove call (#34803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Consider a UnbindContext to be tree-connected if its in a shadow root Signed-off-by: Simon Wülker * Properly track whether a node is in a shadow tree after removing subtree Signed-off-by: Simon Wülker * Update WPT expectations Signed-off-by: Simon Wülker --------- Signed-off-by: Simon Wülker --- components/script/dom/element.rs | 6 ++- components/script/dom/node.rs | 40 ++++++++++++++----- .../getElementById-dynamic-001.html.ini | 3 -- 3 files changed, 34 insertions(+), 15 deletions(-) delete mode 100644 tests/wpt/meta/shadow-dom/getElementById-dynamic-001.html.ini diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 006d99eb2f1..ca7effd3e09 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -3665,7 +3665,11 @@ impl VirtualMethods for Element { } if let Some(ref value) = *self.id_attribute.borrow() { if let Some(ref shadow_root) = self.containing_shadow_root() { - shadow_root.unregister_element_id(self, value.clone()); + // Only unregister the element id if the node was disconnected from it's shadow root + // (as opposed to the whole shadow tree being disconnected as a whole) + if !self.upcast::().is_in_shadow_tree() { + shadow_root.unregister_element_id(self, value.clone()); + } } else { doc.unregister_element_id(self, value.clone()); } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 09684c5d9e9..aaa25abc3a6 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -316,17 +316,28 @@ impl Node { /// Clean up flags and unbind from tree. pub fn complete_remove_subtree(root: &Node, context: &UnbindContext) { - for node in root.traverse_preorder(ShadowIncluding::Yes) { - // Out-of-document elements never have the descendants flag set. - node.set_flag( - NodeFlags::IS_IN_DOC | - NodeFlags::IS_CONNECTED | - NodeFlags::HAS_DIRTY_DESCENDANTS | - NodeFlags::HAS_SNAPSHOT | - NodeFlags::HANDLED_SNAPSHOT, - false, - ); + // Flags that reset when a node is disconnected + const RESET_FLAGS: NodeFlags = NodeFlags::IS_IN_DOC + .union(NodeFlags::IS_CONNECTED) + .union(NodeFlags::HAS_DIRTY_DESCENDANTS) + .union(NodeFlags::HAS_SNAPSHOT) + .union(NodeFlags::HANDLED_SNAPSHOT); + + for node in root.traverse_preorder(ShadowIncluding::No) { + node.set_flag(RESET_FLAGS | NodeFlags::IS_IN_SHADOW_TREE, false); + + // If the element has a shadow root attached to it then we traverse that as well, + // but without touching the IS_IN_SHADOW_TREE flags of the children + if let Some(shadow_root) = node.downcast::().and_then(Element::shadow_root) { + for node in shadow_root + .upcast::() + .traverse_preorder(ShadowIncluding::Yes) + { + node.set_flag(RESET_FLAGS, false); + } + } } + for node in root.traverse_preorder(ShadowIncluding::Yes) { node.clean_up_style_and_layout_data(); @@ -608,6 +619,11 @@ impl Node { self.flags.get().contains(NodeFlags::IS_CONNECTED) } + /// Return true iff the node's root is a Document or a ShadowRoot + pub fn is_connected_to_tree(&self) -> bool { + self.is_connected() || self.is_in_shadow_tree() + } + /// Returns the type ID of this node. pub fn type_id(&self) -> NodeTypeId { match *self.eventtarget.type_id() { @@ -3559,6 +3575,8 @@ pub struct UnbindContext<'a> { /// The next sibling of the inclusive ancestor that was removed. pub next_sibling: Option<&'a Node>, /// Whether the tree is connected. + /// + /// A tree is connected iff it's root is a Document or a ShadowRoot. pub tree_connected: bool, /// Whether the tree is in doc. pub tree_in_doc: bool, @@ -3577,7 +3595,7 @@ impl<'a> UnbindContext<'a> { parent, prev_sibling, next_sibling, - tree_connected: parent.is_connected(), + tree_connected: parent.is_connected_to_tree(), tree_in_doc: parent.is_in_doc(), } } diff --git a/tests/wpt/meta/shadow-dom/getElementById-dynamic-001.html.ini b/tests/wpt/meta/shadow-dom/getElementById-dynamic-001.html.ini deleted file mode 100644 index 40e97cecd7e..00000000000 --- a/tests/wpt/meta/shadow-dom/getElementById-dynamic-001.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[getElementById-dynamic-001.html] - [ShadowRoot.getElementById keeps working after host has been removed] - expected: FAIL