Don't register unconnected shadow roots with their owner document (#34361)

* Don't falsely register Shadow Roots as connected

Previously, a shadowroot would be registered as connected
during the shadow hosts bind_to_tree call, even if the host
was being bound to an element that was not itself
connected to a document.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Update WPT expectations

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Move bind/unbind methods into a VirtualMethod impl

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Add DocumentFragment/Shadowroot to vtable_for

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

components/script/dom/documentfragment.rs

@@ -20,6 +20,7 @@ use crate::dom::element::Element;
 use crate::dom::htmlcollection::HTMLCollection;
 use crate::dom::node::{window_from_node, Node};
 use crate::dom::nodelist::NodeList;
+use crate::dom::virtualmethods::VirtualMethods;
 use crate::dom::window::Window;
 use crate::script_runtime::CanGc;
 
@@ -132,3 +133,9 @@ impl DocumentFragmentMethods<crate::DomTypeHolder> for DocumentFragment {
         self.upcast::<Node>().query_selector_all(selectors)
     }
 }
+
+impl VirtualMethods for DocumentFragment {
+    fn super_type(&self) -> Option<&dyn VirtualMethods> {
+        Some(self.upcast::<Node>() as &dyn VirtualMethods)
+    }
+}

components/script/dom/element.rs

@@ -552,9 +552,11 @@ impl Element {
             .upcast::<Node>()
             .set_containing_shadow_root(Some(&shadow_root));
 
-        if self.is_connected() {
+        let bind_context = BindContext {
-            self.node.owner_doc().register_shadow_root(&shadow_root);
+            tree_connected: self.upcast::<Node>().is_connected(),
-        }
+            tree_in_doc: self.upcast::<Node>().is_in_doc(),
+        };
+        shadow_root.bind_to_tree(&bind_context);
 
         self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
 
@@ -3586,13 +3588,7 @@ impl VirtualMethods for Element {
         let doc = document_from_node(self);
 
         if let Some(ref shadow_root) = self.shadow_root() {
-            doc.register_shadow_root(shadow_root);
+            shadow_root.bind_to_tree(context);
-            let shadow_root = shadow_root.upcast::<Node>();
-            shadow_root.set_flag(NodeFlags::IS_CONNECTED, context.tree_connected);
-            for node in shadow_root.children() {
-                node.set_flag(NodeFlags::IS_CONNECTED, context.tree_connected);
-                node.bind_to_tree(context);
-            }
         }
 
         if !context.tree_connected {
@@ -3637,13 +3633,7 @@ impl VirtualMethods for Element {
         let doc = document_from_node(self);
 
         if let Some(ref shadow_root) = self.shadow_root() {
-            doc.unregister_shadow_root(shadow_root);
+            shadow_root.unbind_from_tree(context);
-            let shadow_root = shadow_root.upcast::<Node>();
-            shadow_root.set_flag(NodeFlags::IS_CONNECTED, false);
-            for node in shadow_root.children() {
-                node.set_flag(NodeFlags::IS_CONNECTED, false);
-                node.unbind_from_tree(context);
-            }
         }
 
         let fullscreen = doc.GetFullscreenElement();

components/script/dom/shadowroot.rs

@@ -24,8 +24,11 @@ use crate::dom::document::Document;
 use crate::dom::documentfragment::DocumentFragment;
 use crate::dom::documentorshadowroot::{DocumentOrShadowRoot, StyleSheetInDocument};
 use crate::dom::element::Element;
-use crate::dom::node::{Node, NodeDamage, NodeFlags, ShadowIncluding, UnbindContext};
+use crate::dom::node::{
+    document_from_node, BindContext, Node, NodeDamage, NodeFlags, ShadowIncluding, UnbindContext,
+};
 use crate::dom::stylesheetlist::{StyleSheetList, StyleSheetListOwner};
+use crate::dom::virtualmethods::VirtualMethods;
 use crate::dom::window::Window;
 use crate::script_runtime::CanGc;
 use crate::stylesheet_set::StylesheetSetRef;
@@ -280,6 +283,48 @@ impl ShadowRootMethods<crate::DomTypeHolder> for ShadowRoot {
     }
 }
 
+impl VirtualMethods for ShadowRoot {
+    fn super_type(&self) -> Option<&dyn VirtualMethods> {
+        Some(self.upcast::<DocumentFragment>() as &dyn VirtualMethods)
+    }
+
+    fn bind_to_tree(&self, context: &BindContext) {
+        if let Some(s) = self.super_type() {
+            s.bind_to_tree(context);
+        }
+
+        if context.tree_connected {
+            let document = document_from_node(self);
+            document.register_shadow_root(self);
+        }
+
+        let shadow_root = self.upcast::<Node>();
+        shadow_root.set_flag(NodeFlags::IS_CONNECTED, context.tree_connected);
+        for node in shadow_root.children() {
+            node.set_flag(NodeFlags::IS_CONNECTED, context.tree_connected);
+            node.bind_to_tree(context);
+        }
+    }
+
+    fn unbind_from_tree(&self, context: &UnbindContext) {
+        if let Some(s) = self.super_type() {
+            s.unbind_from_tree(context);
+        }
+
+        if context.tree_connected {
+            let document = document_from_node(self);
+            document.unregister_shadow_root(self);
+        }
+
+        let shadow_root = self.upcast::<Node>();
+        shadow_root.set_flag(NodeFlags::IS_CONNECTED, false);
+        for node in shadow_root.children() {
+            node.set_flag(NodeFlags::IS_CONNECTED, false);
+            node.unbind_from_tree(context);
+        }
+    }
+}
+
 #[allow(unsafe_code)]
 pub trait LayoutShadowRootHelpers<'dom> {
     fn get_host_for_layout(self) -> LayoutDom<'dom, Element>;

components/script/dom/virtualmethods.rs

@@ -5,14 +5,14 @@
 use html5ever::LocalName;
 use style::attr::AttrValue;
 
-use super::htmltablecolelement::HTMLTableColElement;
 use crate::dom::attr::Attr;
 use crate::dom::bindings::inheritance::{
-    Castable, ElementTypeId, HTMLElementTypeId, HTMLMediaElementTypeId, NodeTypeId,
+    Castable, DocumentFragmentTypeId, ElementTypeId, HTMLElementTypeId, HTMLMediaElementTypeId,
-    SVGElementTypeId, SVGGraphicsElementTypeId,
+    NodeTypeId, SVGElementTypeId, SVGGraphicsElementTypeId,
 };
 use crate::dom::bindings::str::DOMString;
 use crate::dom::document::Document;
+use crate::dom::documentfragment::DocumentFragment;
 use crate::dom::element::{AttributeMutation, Element};
 use crate::dom::event::Event;
 use crate::dom::htmlanchorelement::HTMLAnchorElement;
@@ -46,6 +46,7 @@ use crate::dom::htmlselectelement::HTMLSelectElement;
 use crate::dom::htmlsourceelement::HTMLSourceElement;
 use crate::dom::htmlstyleelement::HTMLStyleElement;
 use crate::dom::htmltablecellelement::HTMLTableCellElement;
+use crate::dom::htmltablecolelement::HTMLTableColElement;
 use crate::dom::htmltableelement::HTMLTableElement;
 use crate::dom::htmltablerowelement::HTMLTableRowElement;
 use crate::dom::htmltablesectionelement::HTMLTableSectionElement;
@@ -54,6 +55,7 @@ use crate::dom::htmltextareaelement::HTMLTextAreaElement;
 use crate::dom::htmltitleelement::HTMLTitleElement;
 use crate::dom::htmlvideoelement::HTMLVideoElement;
 use crate::dom::node::{BindContext, ChildrenMutation, CloneChildrenFlag, Node, UnbindContext};
+use crate::dom::shadowroot::ShadowRoot;
 use crate::dom::svgelement::SVGElement;
 use crate::dom::svgsvgelement::SVGSVGElement;
 
@@ -283,6 +285,12 @@ pub fn vtable_for(node: &Node) -> &dyn VirtualMethods {
             node.downcast::<Element>().unwrap() as &dyn VirtualMethods
         },
         NodeTypeId::Element(_) => node.downcast::<HTMLElement>().unwrap() as &dyn VirtualMethods,
+        NodeTypeId::DocumentFragment(DocumentFragmentTypeId::ShadowRoot) => {
+            node.downcast::<ShadowRoot>().unwrap() as &dyn VirtualMethods
+        },
+        NodeTypeId::DocumentFragment(_) => {
+            node.downcast::<DocumentFragment>().unwrap() as &dyn VirtualMethods
+        },
         _ => node as &dyn VirtualMethods,
     }
 }

tests/wpt/meta/custom-elements/CustomElementRegistry.html.ini

@@ -1,5 +1,4 @@
 [CustomElementRegistry.html]
-  expected: CRASH
   [customElements.define must upgrade elements in the shadow-including tree order]
     expected: FAIL
 

tests/wpt/meta/custom-elements/custom-element-registry/upgrade.html.ini

@@ -1,4 +1,3 @@
 [upgrade.html]
-  expected: CRASH
   [Two elements as shadow-including descendants (and not descendants) of the upgraded node]
     expected: FAIL

tests/wpt/meta/html/semantics/forms/the-input-element/radio-disconnected-group-owner.html.ini

@@ -1,5 +1,4 @@
 [radio-disconnected-group-owner.html]
-  expected: CRASH
   [Removed elements are moved into separate radio groups.]
     expected: FAIL
 

tests/wpt/meta/html/semantics/interactive-elements/the-dialog-element/dialog-focus-shadow-double-nested.html.ini

@@ -1,2 +1,6 @@
 [dialog-focus-shadow-double-nested.html]
-  expected: CRASH
+  [show()]
+    expected: FAIL
+
+  [showModal()]
+    expected: FAIL

tests/wpt/meta/shadow-dom/Extensions-to-Event-Interface.html.ini

@@ -1,2 +1,33 @@
 [Extensions-to-Event-Interface.html]
-  expected: CRASH
+  [composedPath() must return an empty array when the event is no longer dispatched]
+    expected: FAIL
+
+  [composed on EventInit must default to false]
+    expected: FAIL
+
+  [composed on EventInit must set the composed flag]
+    expected: FAIL
+
+  [The event must propagate out of open mode shadow boundaries when the composed flag is set]
+    expected: FAIL
+
+  [The event must propagate out of closed mode shadow boundaries when the composed flag is set]
+    expected: FAIL
+
+  [The event must not propagate out of open mode shadow tree of the target but must propagate out of inner shadow trees when the scoped flag is set]
+    expected: FAIL
+
+  [The event must not propagate out of closed mode shadow tree of the target but must propagate out of inner shadow trees when the scoped flag is set]
+    expected: FAIL
+
+  [The event must propagate out of open mode shadow tree in which the relative target and the relative related target are the same]
+    expected: FAIL
+
+  [The event must propagate out of closed mode shadow tree in which the relative target and the relative related target are the same]
+    expected: FAIL
+
+  [composedPath() must contain and only contain the unclosed nodes of target in open mode shadow trees]
+    expected: FAIL
+
+  [composedPath() must contain and only contain the unclosed nodes of target in closed mode shadow trees]
+    expected: FAIL

tests/wpt/meta/shadow-dom/event-with-related-target.html.ini

@@ -1,2 +1,54 @@
 [event-with-related-target.html]
-  expected: CRASH
+  [Firing an event at B1a with relatedNode at B1 with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at B1 with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at B1b1 with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at B1b1 with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1b1 with relatedNode at B1a with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1b1 with relatedNode at B1a with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at D1 with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at D1 with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at D1 with relatedNode at B1a with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at D1 with relatedNode at B1a with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at A1a with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at A1a with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at A1a with relatedNode at B1a with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at A1a with relatedNode at B1a with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at A1a (detached) with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at B1a with relatedNode at A1a (detached) with closed mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at A1a with relatedNode at B1a (detached) with open mode shadow trees]
+    expected: FAIL
+
+  [Firing an event at A1a with relatedNode at B1a (detached) with closed mode shadow trees]
+    expected: FAIL