diff options
6 files changed, 167 insertions, 67 deletions
diff --git a/components/script/dom/htmlformcontrolscollection.rs b/components/script/dom/htmlformcontrolscollection.rs index 05ea6426b27..8bf296a8417 100644 --- a/components/script/dom/htmlformcontrolscollection.rs +++ b/components/script/dom/htmlformcontrolscollection.rs @@ -6,16 +6,17 @@ use crate::dom::bindings::codegen::Bindings::HTMLCollectionBinding::HTMLCollecti use crate::dom::bindings::codegen::Bindings::HTMLFormControlsCollectionBinding; use crate::dom::bindings::codegen::Bindings::HTMLFormControlsCollectionBinding::HTMLFormControlsCollectionMethods; use crate::dom::bindings::codegen::UnionTypes::RadioNodeListOrElement; +use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::reflector::{reflect_dom_object, DomObject}; use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; use crate::dom::element::Element; use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection}; +use crate::dom::htmlformelement::HTMLFormElement; use crate::dom::node::Node; use crate::dom::radionodelist::RadioNodeList; use crate::dom::window::Window; use dom_struct::dom_struct; -use std::iter; #[dom_struct] pub struct HTMLFormControlsCollection { @@ -24,17 +25,17 @@ pub struct HTMLFormControlsCollection { impl HTMLFormControlsCollection { fn new_inherited( - root: &Node, + root: &HTMLFormElement, filter: Box<dyn CollectionFilter + 'static>, ) -> HTMLFormControlsCollection { HTMLFormControlsCollection { - collection: HTMLCollection::new_inherited(root, filter), + collection: HTMLCollection::new_inherited(root.upcast::<Node>(), filter), } } pub fn new( window: &Window, - root: &Node, + root: &HTMLFormElement, filter: Box<dyn CollectionFilter + 'static>, ) -> DomRoot<HTMLFormControlsCollection> { reflect_dom_object( @@ -76,12 +77,16 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection { Some(RadioNodeListOrElement::Element(elem)) } else { // Step 4-5 - let once = iter::once(DomRoot::upcast::<Node>(elem)); - let list = once.chain(peekable.map(DomRoot::upcast)); let global = self.global(); let window = global.as_window(); + // okay to unwrap: root's type was checked in the constructor + let collection_root = self.collection.root_node(); + let form = collection_root.downcast::<HTMLFormElement>().unwrap(); + // There is only one way to get an HTMLCollection, + // specifically HTMLFormElement::Elements(), + // and the collection filter excludes image inputs. Some(RadioNodeListOrElement::RadioNodeList( - RadioNodeList::new_simple_list(window, list), + RadioNodeList::new_controls_except_image_inputs(window, form, name), )) } // Step 3 diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs index b788c113b8e..c36f30c1fb3 100644 --- a/components/script/dom/htmlformelement.rs +++ b/components/script/dom/htmlformelement.rs @@ -13,6 +13,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLFormElementBinding; use crate::dom::bindings::codegen::Bindings::HTMLFormElementBinding::HTMLFormElementMethods; use crate::dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods; use crate::dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding::HTMLTextAreaElementMethods; +use crate::dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods; use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods; use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId}; use crate::dom::bindings::refcounted::Trusted; @@ -45,6 +46,8 @@ use crate::dom::htmltextareaelement::HTMLTextAreaElement; use crate::dom::node::{document_from_node, window_from_node}; use crate::dom::node::{Node, NodeFlags, ShadowIncluding}; use crate::dom::node::{UnbindContext, VecPreOrderInsertionHelper}; +use crate::dom::nodelist::{NodeList, RadioListMode}; +use crate::dom::radionodelist::RadioNodeList; use crate::dom::validitystate::ValidationFlags; use crate::dom::virtualmethods::VirtualMethods; use crate::dom::window::Window; @@ -65,7 +68,6 @@ use style::attr::AttrValue; use style::str::split_html_space_chars; use crate::dom::bindings::codegen::UnionTypes::RadioNodeListOrElement; -use crate::dom::radionodelist::RadioNodeList; use std::collections::HashMap; use time::{now, Duration, Tm}; @@ -115,6 +117,69 @@ impl HTMLFormElement { HTMLFormElementBinding::Wrap, ) } + + fn filter_for_radio_list(mode: RadioListMode, child: &Element, name: &DOMString) -> bool { + if let Some(child) = child.downcast::<Element>() { + match mode { + RadioListMode::ControlsExceptImageInputs => { + if child + .downcast::<HTMLElement>() + .map_or(false, |c| c.is_listed_element()) + { + if (child.has_attribute(&local_name!("id")) && + child.get_string_attribute(&local_name!("id")) == *name) || + (child.has_attribute(&local_name!("name")) && + child.get_string_attribute(&local_name!("name")) == *name) + { + if let Some(inp) = child.downcast::<HTMLInputElement>() { + // input, only return it if it's not image-button state + return inp.input_type() != InputType::Image; + } else { + // control, but not an input + return true; + } + } + } + return false; + }, + RadioListMode::Images => { + if child.is::<HTMLImageElement>() { + if (child.has_attribute(&local_name!("id")) && + child.get_string_attribute(&local_name!("id")) == *name) || + (child.has_attribute(&local_name!("name")) && + child.get_string_attribute(&local_name!("name")) == *name) + { + return true; + } + } + return false; + }, + } + } + false + } + + pub fn nth_for_radio_list( + &self, + index: u32, + mode: RadioListMode, + name: &DOMString, + ) -> Option<DomRoot<Node>> { + self.controls + .borrow() + .iter() + .filter(|n| HTMLFormElement::filter_for_radio_list(mode, &*n, name)) + .nth(index as usize) + .and_then(|n| Some(DomRoot::from_ref(n.upcast::<Node>()))) + } + + pub fn count_for_radio_list(&self, mode: RadioListMode, name: &DOMString) -> u32 { + self.controls + .borrow() + .iter() + .filter(|n| HTMLFormElement::filter_for_radio_list(mode, &**n, name)) + .count() as u32 + } } impl HTMLFormElementMethods for HTMLFormElement { @@ -248,7 +313,7 @@ impl HTMLFormElementMethods for HTMLFormElement { form: DomRoot::from_ref(self), }); let window = window_from_node(self); - HTMLFormControlsCollection::new(&window, self.upcast(), filter) + HTMLFormControlsCollection::new(&window, self, filter) })) } @@ -265,43 +330,23 @@ impl HTMLFormElementMethods for HTMLFormElement { // https://html.spec.whatwg.org/multipage/#the-form-element%3Adetermine-the-value-of-a-named-property fn NamedGetter(&self, name: DOMString) -> Option<RadioNodeListOrElement> { - let mut candidates: Vec<DomRoot<Node>> = Vec::new(); + let window = window_from_node(self); - let controls = self.controls.borrow(); // Step 1 - for child in controls.iter() { - if child - .downcast::<HTMLElement>() - .map_or(false, |c| c.is_listed_element()) - { - if (child.has_attribute(&local_name!("id")) && - child.get_string_attribute(&local_name!("id")) == name) || - (child.has_attribute(&local_name!("name")) && - child.get_string_attribute(&local_name!("name")) == name) - { - candidates.push(DomRoot::from_ref(&*child.upcast::<Node>())); - } - } - } + let mut candidates = + RadioNodeList::new_controls_except_image_inputs(&window, self, name.clone()); + let mut candidates_length = candidates.Length(); + // Step 2 - if candidates.len() == 0 { - for child in controls.iter() { - if child.is::<HTMLImageElement>() { - if (child.has_attribute(&local_name!("id")) && - child.get_string_attribute(&local_name!("id")) == name) || - (child.has_attribute(&local_name!("name")) && - child.get_string_attribute(&local_name!("name")) == name) - { - candidates.push(DomRoot::from_ref(&*child.upcast::<Node>())); - } - } - } + if candidates_length == 0 { + candidates = RadioNodeList::new_images(&window, self, name.clone()); + candidates_length = candidates.Length(); } let mut past_names_map = self.past_names_map.borrow_mut(); // Step 3 - if candidates.len() == 0 { + if candidates_length == 0 { if past_names_map.contains_key(&name) { return Some(RadioNodeListOrElement::Element(DomRoot::from_ref( &*past_names_map.get(&name).unwrap().0, @@ -311,16 +356,13 @@ impl HTMLFormElementMethods for HTMLFormElement { } // Step 4 - if candidates.len() > 1 { - let window = window_from_node(self); - - return Some(RadioNodeListOrElement::RadioNodeList( - RadioNodeList::new_simple_list(&window, candidates.into_iter()), - )); + if candidates_length > 1 { + return Some(RadioNodeListOrElement::RadioNodeList(candidates)); } // Step 5 - let element_node = &candidates[0]; + // candidates_length is 1, so we can unwrap item 0 + let element_node = candidates.upcast::<NodeList>().Item(0).unwrap(); past_names_map.insert( name, ( diff --git a/components/script/dom/nodelist.rs b/components/script/dom/nodelist.rs index 88fdd4cb70c..1fdcef4b340 100644 --- a/components/script/dom/nodelist.rs +++ b/components/script/dom/nodelist.rs @@ -7,7 +7,9 @@ use crate::dom::bindings::codegen::Bindings::NodeListBinding; use crate::dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods; use crate::dom::bindings::reflector::{reflect_dom_object, Reflector}; use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom}; +use crate::dom::bindings::str::DOMString; use crate::dom::htmlelement::HTMLElement; +use crate::dom::htmlformelement::HTMLFormElement; use crate::dom::node::{ChildrenMutation, Node}; use crate::dom::window::Window; use dom_struct::dom_struct; @@ -19,6 +21,7 @@ pub enum NodeListType { Simple(Vec<Dom<Node>>), Children(ChildrenList), Labels(LabelsList), + Radio(RadioList), } // https://dom.spec.whatwg.org/#interface-nodelist @@ -83,6 +86,7 @@ impl NodeListMethods for NodeList { NodeListType::Simple(ref elems) => elems.len() as u32, NodeListType::Children(ref list) => list.len(), NodeListType::Labels(ref list) => list.len(), + NodeListType::Radio(ref list) => list.len(), } } @@ -94,6 +98,7 @@ impl NodeListMethods for NodeList { .map(|node| DomRoot::from_ref(&**node)), NodeListType::Children(ref list) => list.item(index), NodeListType::Labels(ref list) => list.item(index), + NodeListType::Radio(ref list) => list.item(index), } } @@ -108,20 +113,22 @@ impl NodeList { if let NodeListType::Children(ref list) = self.list_type { list } else { - panic!("called as_children_list() on a simple node list") + panic!("called as_children_list() on a non-children node list") } } - pub fn as_simple_list(&self) -> &Vec<Dom<Node>> { - if let NodeListType::Simple(ref list) = self.list_type { + pub fn as_radio_list(&self) -> &RadioList { + if let NodeListType::Radio(ref list) = self.list_type { list } else { - panic!("called as_simple_list() on a children node list") + panic!("called as_radio_list() on a non-radio node list") } } pub fn iter<'a>(&'a self) -> impl Iterator<Item = DomRoot<Node>> + 'a { let len = self.Length(); + // There is room for optimization here in non-simple cases, + // as calling Item repeatedly on a live list can involve redundant work. (0..len).flat_map(move |i| self.Item(i)) } } @@ -328,7 +335,7 @@ impl ChildrenList { } } -// Labels lists: There might room for performance optimization +// Labels lists: There might be room for performance optimization // analogous to the ChildrenMutation case of a children list, // in which we can keep information from an older access live // if we know nothing has happened that would change it. @@ -357,3 +364,40 @@ impl LabelsList { self.element.label_at(index) } } + +// Radio node lists: There is room for performance improvement here; +// a form is already aware of changes to its set of controls, +// so a radio list can cache and cache-invalidate its contents +// just by hooking into what the form already knows without a +// separate mutation observer. FIXME #25482 +#[derive(Clone, Copy, JSTraceable, MallocSizeOf)] +pub enum RadioListMode { + ControlsExceptImageInputs, + Images, +} + +#[derive(JSTraceable, MallocSizeOf)] +#[unrooted_must_root_lint::must_root] +pub struct RadioList { + form: Dom<HTMLFormElement>, + mode: RadioListMode, + name: DOMString, +} + +impl RadioList { + pub fn new(form: &HTMLFormElement, mode: RadioListMode, name: DOMString) -> RadioList { + RadioList { + form: Dom::from_ref(form), + mode: mode, + name: name, + } + } + + pub fn len(&self) -> u32 { + self.form.count_for_radio_list(self.mode, &self.name) + } + + pub fn item(&self, index: u32) -> Option<DomRoot<Node>> { + self.form.nth_for_radio_list(index, self.mode, &self.name) + } +} diff --git a/components/script/dom/radionodelist.rs b/components/script/dom/radionodelist.rs index d178921056a..21b7d39772b 100644 --- a/components/script/dom/radionodelist.rs +++ b/components/script/dom/radionodelist.rs @@ -8,11 +8,12 @@ use crate::dom::bindings::codegen::Bindings::RadioNodeListBinding; use crate::dom::bindings::codegen::Bindings::RadioNodeListBinding::RadioNodeListMethods; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::reflector::reflect_dom_object; -use crate::dom::bindings::root::{Dom, DomRoot}; +use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; +use crate::dom::htmlformelement::HTMLFormElement; use crate::dom::htmlinputelement::{HTMLInputElement, InputType}; use crate::dom::node::Node; -use crate::dom::nodelist::{NodeList, NodeListType}; +use crate::dom::nodelist::{NodeList, NodeListType, RadioList, RadioListMode}; use crate::dom::window::Window; use dom_struct::dom_struct; @@ -38,18 +39,33 @@ impl RadioNodeList { ) } - pub fn new_simple_list<T>(window: &Window, iter: T) -> DomRoot<RadioNodeList> - where - T: Iterator<Item = DomRoot<Node>>, - { + pub fn new_controls_except_image_inputs( + window: &Window, + form: &HTMLFormElement, + name: DOMString, + ) -> DomRoot<RadioNodeList> { RadioNodeList::new( window, - NodeListType::Simple(iter.map(|r| Dom::from_ref(&*r)).collect()), + NodeListType::Radio(RadioList::new( + form, + RadioListMode::ControlsExceptImageInputs, + name, + )), ) } - // FIXME: This shouldn't need to be implemented here since NodeList (the parent of - // RadioNodeList) implements Length + pub fn new_images( + window: &Window, + form: &HTMLFormElement, + name: DOMString, + ) -> DomRoot<RadioNodeList> { + RadioNodeList::new( + window, + NodeListType::Radio(RadioList::new(form, RadioListMode::Images, name)), + ) + } + + // https://dom.spec.whatwg.org/#dom-nodelist-length // https://github.com/servo/servo/issues/5875 pub fn Length(&self) -> u32 { self.node_list.Length() @@ -60,7 +76,6 @@ impl RadioNodeListMethods for RadioNodeList { // https://html.spec.whatwg.org/multipage/#dom-radionodelist-value fn Value(&self) -> DOMString { self.upcast::<NodeList>() - .as_simple_list() .iter() .filter_map(|node| { // Step 1 @@ -85,7 +100,7 @@ impl RadioNodeListMethods for RadioNodeList { // https://html.spec.whatwg.org/multipage/#dom-radionodelist-value fn SetValue(&self, value: DOMString) { - for node in self.upcast::<NodeList>().as_simple_list().iter() { + for node in self.upcast::<NodeList>().iter() { // Step 1 if let Some(input) = node.downcast::<HTMLInputElement>() { match input.input_type() { diff --git a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-elements-nameditem-01.html.ini b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-elements-nameditem-01.html.ini index 84bdf90a6cb..f90f0a7ada5 100644 --- a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-elements-nameditem-01.html.ini +++ b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-elements-nameditem-01.html.ini @@ -1,8 +1,5 @@ [form-elements-nameditem-01.html] type: testharness - [elements collection should return elements or RadioNodeLists] - expected: FAIL - [elements collection should include fieldsets] expected: FAIL diff --git a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini index b33041d0dc6..ee74e3e9258 100644 --- a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini +++ b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini @@ -1,8 +1,5 @@ [form-nameditem.html] type: testharness - [All listed elements except input type=image should be present in the form] - expected: FAIL - [Named elements should override builtins] expected: FAIL |