Labels are a live list in tree order

components/script/dom/htmlbuttonelement.rs

@@ -41,6 +41,7 @@ pub struct HTMLButtonElement {
     htmlelement: HTMLElement,
     button_type: Cell<ButtonType>,
     form_owner: MutNullableDom<HTMLFormElement>,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 impl HTMLButtonElement {
@@ -58,6 +59,7 @@ impl HTMLButtonElement {
             ),
             button_type: Cell::new(ButtonType::Submit),
             form_owner: Default::default(),
+            labels_node_list: Default::default(),
         }
     }
 
@@ -149,9 +151,7 @@ impl HTMLButtonElementMethods for HTMLButtonElement {
     make_setter!(SetValue, "value");
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 }
 
 impl HTMLButtonElement {

components/script/dom/htmlelement.rs

@@ -4,10 +4,10 @@
 
 use crate::dom::activation::{synthetic_click_activation, ActivationSource};
 use crate::dom::attr::Attr;
-use crate::dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
 use crate::dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use crate::dom::bindings::codegen::Bindings::HTMLElementBinding;
 use crate::dom::bindings::codegen::Bindings::HTMLElementBinding::HTMLElementMethods;
+use crate::dom::bindings::codegen::Bindings::HTMLLabelElementBinding::HTMLLabelElementMethods;
 use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeBinding::NodeMethods;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use crate::dom::bindings::error::{Error, ErrorResult};
@@ -29,7 +29,6 @@ use crate::dom::htmlinputelement::{HTMLInputElement, InputType};
 use crate::dom::htmllabelelement::HTMLLabelElement;
 use crate::dom::node::{document_from_node, window_from_node};
 use crate::dom::node::{BindContext, Node, NodeFlags, ShadowIncluding};
-use crate::dom::nodelist::NodeList;
 use crate::dom::text::Text;
 use crate::dom::virtualmethods::VirtualMethods;
 use dom_struct::dom_struct;
@@ -677,43 +676,48 @@ impl HTMLElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    pub fn labels(&self) -> DomRoot<NodeList> {
-        debug_assert!(self.is_labelable_element());
-
+    // This gets the nth label in tree order.
+    pub fn label_at(&self, index: u32) -> Option<DomRoot<Node>> {
         let element = self.upcast::<Element>();
-        let window = window_from_node(element);
-
-        // Traverse ancestors for implicitly associated <label> elements
-        // https://html.spec.whatwg.org/multipage/#the-label-element:attr-label-for-4
-        let ancestors = self
-            .upcast::<Node>()
-            .ancestors()
-            .filter_map(DomRoot::downcast::<HTMLElement>)
-            // If we reach a labelable element, we have a guarantee no ancestors above it
-            // will be a label for this HTMLElement
-            .take_while(|elem| !elem.is_labelable_element())
-            .filter_map(DomRoot::downcast::<HTMLLabelElement>)
-            .filter(|elem| !elem.upcast::<Element>().has_attribute(&local_name!("for")))
-            .filter(|elem| elem.first_labelable_descendant().as_deref() == Some(self))
-            .map(DomRoot::upcast::<Node>);
 
-        let id = element.Id();
-        let id = match &id as &str {
-            "" => return NodeList::new_simple_list(&window, ancestors),
-            id => id,
-        };
-
-        // Traverse entire tree for <label> elements with `for` attribute matching `id`
+        // Traverse entire tree for <label> elements that have
+        // this as their control.
+        // There is room for performance optimization, as we don't need
+        // the actual result of GetControl, only whether the result
+        // would match self.
+        // (Even more room for performance optimization: do what
+        // nodelist ChildrenList does and keep a mutation-aware cursor
+        // around; this may be hard since labels need to keep working
+        // even as they get detached into a subtree and reattached to
+        // a document.)
         let root_element = element.root_element();
         let root_node = root_element.upcast::<Node>();
-        let children = root_node
+        root_node
             .traverse_preorder(ShadowIncluding::No)
-            .filter_map(DomRoot::downcast::<Element>)
-            .filter(|elem| elem.is::<HTMLLabelElement>())
-            .filter(|elem| elem.get_string_attribute(&local_name!("for")) == id)
-            .map(DomRoot::upcast::<Node>);
+            .filter_map(DomRoot::downcast::<HTMLLabelElement>)
+            .filter(|elem| match elem.GetControl() {
+                Some(control) => &*control == self,
+                _ => false,
+            })
+            .nth(index as usize)
+            .map(|n| DomRoot::from_ref(n.upcast::<Node>()))
+    }
 
-        NodeList::new_simple_list(&window, children.chain(ancestors))
+    // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
+    // This counts the labels of the element, to support NodeList::Length
+    pub fn labels_count(&self) -> u32 {
+        // see label_at comments about performance
+        let element = self.upcast::<Element>();
+        let root_element = element.root_element();
+        let root_node = root_element.upcast::<Node>();
+        root_node
+            .traverse_preorder(ShadowIncluding::No)
+            .filter_map(DomRoot::downcast::<HTMLLabelElement>)
+            .filter(|elem| match elem.GetControl() {
+                Some(control) => &*control == self,
+                _ => false,
+            })
+            .count() as u32
     }
 }
 

components/script/dom/htmlinputelement.rs

@@ -234,6 +234,7 @@ pub struct HTMLInputElement {
 
     filelist: MutNullableDom<FileList>,
     form_owner: MutNullableDom<HTMLFormElement>,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 #[derive(JSTraceable)]
@@ -303,6 +304,7 @@ impl HTMLInputElement {
             value_dirty: Cell::new(false),
             filelist: MutNullableDom::new(None),
             form_owner: Default::default(),
+            labels_node_list: MutNullableDom::new(None),
         }
     }
 
@@ -791,12 +793,18 @@ impl HTMLInputElementMethods for HTMLInputElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
+    // Different from make_labels_getter because this one
+    // conditionally returns null.
+    fn GetLabels(&self) -> Option<DomRoot<NodeList>> {
         if self.input_type() == InputType::Hidden {
-            let window = window_from_node(self);
-            NodeList::empty(&window)
+            None
         } else {
-            self.upcast::<HTMLElement>().labels()
+            Some(self.labels_node_list.or_init(|| {
+                NodeList::new_labels_list(
+                    self.upcast::<Node>().owner_doc().window(),
+                    self.upcast::<HTMLElement>(),
+                )
+            }))
         }
     }
 

components/script/dom/htmllabelelement.rs

@@ -4,8 +4,11 @@
 
 use crate::dom::activation::{synthetic_click_activation, Activatable, ActivationSource};
 use crate::dom::attr::Attr;
+use crate::dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
+use crate::dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
 use crate::dom::bindings::codegen::Bindings::HTMLLabelElementBinding;
 use crate::dom::bindings::codegen::Bindings::HTMLLabelElementBinding::HTMLLabelElementMethods;
+use crate::dom::bindings::codegen::Bindings::NodeBinding::{GetRootNodeOptions, NodeMethods};
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::root::DomRoot;
 use crate::dom::bindings::str::DOMString;
@@ -15,7 +18,7 @@ use crate::dom::event::Event;
 use crate::dom::eventtarget::EventTarget;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::htmlformelement::{FormControl, FormControlElementHelpers, HTMLFormElement};
-use crate::dom::node::{document_from_node, Node, ShadowIncluding};
+use crate::dom::node::{Node, ShadowIncluding};
 use crate::dom::virtualmethods::VirtualMethods;
 use dom_struct::dom_struct;
 use html5ever::{LocalName, Prefix};
@@ -99,10 +102,6 @@ impl HTMLLabelElementMethods for HTMLLabelElement {
 
     // https://html.spec.whatwg.org/multipage/#dom-label-control
     fn GetControl(&self) -> Option<DomRoot<HTMLElement>> {
-        if !self.upcast::<Node>().is_in_doc() {
-            return None;
-        }
-
         let for_attr = match self
             .upcast::<Element>()
             .get_attribute(&ns!(), &local_name!("for"))
@@ -111,13 +110,40 @@ impl HTMLLabelElementMethods for HTMLLabelElement {
             None => return self.first_labelable_descendant(),
         };
 
-        let for_value = for_attr.value();
-        document_from_node(self)
-            .get_element_by_id(for_value.as_atom())
-            .and_then(DomRoot::downcast::<HTMLElement>)
-            .into_iter()
-            .filter(|e| e.is_labelable_element())
-            .next()
+        let for_value = for_attr.Value();
+
+        // "If the attribute is specified and there is an element in the tree
+        // whose ID is equal to the value of the for attribute, and the first
+        // such element in tree order is a labelable element, then that
+        // element is the label element's labeled control."
+        // Two subtle points here: we need to search the _tree_, which is
+        // not necessarily the document if we're detached from the document,
+        // and we only consider one element even if a later element with
+        // the same ID is labelable.
+
+        let maybe_found = self
+            .upcast::<Node>()
+            .GetRootNode(&GetRootNodeOptions::empty())
+            .traverse_preorder(ShadowIncluding::No)
+            .find_map(|e| {
+                if let Some(htmle) = e.downcast::<HTMLElement>() {
+                    if htmle.upcast::<Element>().Id() == for_value {
+                        Some(DomRoot::from_ref(htmle))
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            });
+        // We now have the element that we would return, but only return it
+        // if it's labelable.
+        if let Some(ref maybe_labelable) = maybe_found {
+            if maybe_labelable.is_labelable_element() {
+                return maybe_found;
+            }
+        }
+        None
     }
 }
 

components/script/dom/htmlmeterelement.rs

@@ -6,7 +6,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLMeterElementBinding::{
     self, HTMLMeterElementMethods,
 };
 use crate::dom::bindings::inheritance::Castable;
-use crate::dom::bindings::root::DomRoot;
+use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::document::Document;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::node::Node;
@@ -17,6 +17,7 @@ use html5ever::{LocalName, Prefix};
 #[dom_struct]
 pub struct HTMLMeterElement {
     htmlelement: HTMLElement,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 impl HTMLMeterElement {
@@ -27,6 +28,7 @@ impl HTMLMeterElement {
     ) -> HTMLMeterElement {
         HTMLMeterElement {
             htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
+            labels_node_list: MutNullableDom::new(None),
         }
     }
 
@@ -48,7 +50,5 @@ impl HTMLMeterElement {
 
 impl HTMLMeterElementMethods for HTMLMeterElement {
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 }

components/script/dom/htmloutputelement.rs

@@ -22,6 +22,7 @@ use html5ever::{LocalName, Prefix};
 pub struct HTMLOutputElement {
     htmlelement: HTMLElement,
     form_owner: MutNullableDom<HTMLFormElement>,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 impl HTMLOutputElement {
@@ -33,6 +34,7 @@ impl HTMLOutputElement {
         HTMLOutputElement {
             htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
             form_owner: Default::default(),
+            labels_node_list: Default::default(),
         }
     }
 
@@ -65,9 +67,7 @@ impl HTMLOutputElementMethods for HTMLOutputElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 }
 
 impl VirtualMethods for HTMLOutputElement {

components/script/dom/htmlprogresselement.rs

@@ -6,7 +6,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLProgressElementBinding::{
     self, HTMLProgressElementMethods,
 };
 use crate::dom::bindings::inheritance::Castable;
-use crate::dom::bindings::root::DomRoot;
+use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::document::Document;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::node::Node;
@@ -17,6 +17,7 @@ use html5ever::{LocalName, Prefix};
 #[dom_struct]
 pub struct HTMLProgressElement {
     htmlelement: HTMLElement,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 impl HTMLProgressElement {
@@ -27,6 +28,7 @@ impl HTMLProgressElement {
     ) -> HTMLProgressElement {
         HTMLProgressElement {
             htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
+            labels_node_list: MutNullableDom::new(None),
         }
     }
 
@@ -48,7 +50,5 @@ impl HTMLProgressElement {
 
 impl HTMLProgressElementMethods for HTMLProgressElement {
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 }

components/script/dom/htmlselectelement.rs

@@ -61,6 +61,7 @@ pub struct HTMLSelectElement {
     htmlelement: HTMLElement,
     options: MutNullableDom<HTMLOptionsCollection>,
     form_owner: MutNullableDom<HTMLFormElement>,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 static DEFAULT_SELECT_SIZE: u32 = 0;
@@ -80,6 +81,7 @@ impl HTMLSelectElement {
             ),
             options: Default::default(),
             form_owner: Default::default(),
+            labels_node_list: Default::default(),
         }
     }
 
@@ -249,9 +251,7 @@ impl HTMLSelectElementMethods for HTMLSelectElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 
     // https://html.spec.whatwg.org/multipage/#dom-select-options
     fn Options(&self) -> DomRoot<HTMLOptionsCollection> {

components/script/dom/htmltextareaelement.rs

@@ -52,6 +52,7 @@ pub struct HTMLTextAreaElement {
     // https://html.spec.whatwg.org/multipage/#concept-textarea-dirty
     value_dirty: Cell<bool>,
     form_owner: MutNullableDom<HTMLFormElement>,
+    labels_node_list: MutNullableDom<NodeList>,
 }
 
 pub trait LayoutHTMLTextAreaElementHelpers {
@@ -153,6 +154,7 @@ impl HTMLTextAreaElement {
             )),
             value_dirty: Cell::new(false),
             form_owner: Default::default(),
+            labels_node_list: Default::default(),
         }
     }
 
@@ -316,9 +318,7 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-lfe-labels
-    fn Labels(&self) -> DomRoot<NodeList> {
-        self.upcast::<HTMLElement>().labels()
-    }
+    make_labels_getter!(Labels, labels_node_list);
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-select
     fn Select(&self) {