From 01ea761bd0ca13f58c6144bb4a5091c6a6c48568 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 25 Sep 2015 00:04:43 +0200 Subject: [PATCH 1/2] Use an atom for HTMLInputElement's name attribute's value --- components/script/dom/htmlinputelement.rs | 30 +++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 0de98f19d49..8ae1390bf9c 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -291,7 +291,7 @@ impl HTMLInputElementMethods for HTMLInputElement { make_getter!(Name); // https://html.spec.whatwg.org/multipage/#attr-fe-name - make_setter!(SetName, "name"); + make_atomic_setter!(SetName, "name"); // https://html.spec.whatwg.org/multipage/#attr-input-placeholder make_getter!(Placeholder); @@ -337,7 +337,7 @@ impl HTMLInputElementMethods for HTMLInputElement { #[allow(unsafe_code)] -fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&str>) { +fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) { //TODO: if not in document, use root ancestor instead of document let owner = broadcaster.form_owner(); let doc = document_from_node(broadcaster); @@ -345,7 +345,7 @@ fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&str>) // This function is a workaround for lifetime constraint difficulties. fn do_broadcast(doc_node: &Node, broadcaster: &HTMLInputElement, - owner: Option<&HTMLFormElement>, group: Option<&str>) { + owner: Option<&HTMLFormElement>, group: Option<&Atom>) { // There is no DOM tree manipulation here, so this is safe let iter = unsafe { doc_node.query_selector_iter("input[type=radio]".to_owned()).unwrap() @@ -363,7 +363,7 @@ fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&str>) } fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>, - group: Option<&str>) -> bool { + group: Option<&Atom>) -> bool { let other_owner = other.form_owner(); let other_owner = other_owner.r(); other.input_type.get() == InputType::InputRadio && @@ -371,7 +371,7 @@ fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>, other_owner == owner && // TODO should be a unicode compatibility caseless match match (other.get_radio_group_name(), group) { - (Some(ref s1), Some(s2)) => &**s1 == s2, + (Some(ref s1), Some(s2)) => s1 == s2, (None, None) => true, _ => false } @@ -384,17 +384,18 @@ impl HTMLInputElement { doc.r().content_changed(node, NodeDamage::OtherNodeDamage) } - pub fn radio_group_updated(&self, group: Option<&str>) { + pub fn radio_group_updated(&self, group: Option<&Atom>) { if self.Checked() { broadcast_radio_checked(self, group); } } - pub fn get_radio_group_name(&self) -> Option { + // https://html.spec.whatwg.org/multipage/#radio-button-group + pub fn get_radio_group_name(&self) -> Option { //TODO: determine form owner let elem = ElementCast::from_ref(self); elem.get_attribute(&ns!(""), &atom!("name")) - .map(|name| name.r().Value()) + .map(|name| name.value().as_atom().clone()) } pub fn update_checked_state(&self, checked: bool, dirty: bool) { @@ -406,9 +407,7 @@ impl HTMLInputElement { if self.input_type.get() == InputType::InputRadio && checked { broadcast_radio_checked(self, - self.get_radio_group_name() - .as_ref() - .map(|group| &**group)); + self.get_radio_group_name().as_ref()); } self.force_relayout(); @@ -504,14 +503,14 @@ impl VirtualMethods for HTMLInputElement { self.input_type.set(value); if value == InputType::InputRadio { self.radio_group_updated( - self.get_radio_group_name().as_ref().map(|group| &**group)); + self.get_radio_group_name().as_ref()); } }, AttributeMutation::Removed => { if self.input_type.get() == InputType::InputRadio { broadcast_radio_checked( self, - self.get_radio_group_name().as_ref().map(|group| &**group)); + self.get_radio_group_name().as_ref()); } self.input_type.set(InputType::InputText); } @@ -524,7 +523,7 @@ impl VirtualMethods for HTMLInputElement { }, &atom!(name) if self.input_type.get() == InputType::InputRadio => { self.radio_group_updated( - mutation.new_value(attr).as_ref().map(|value| &***value)); + mutation.new_value(attr).as_ref().map(|name| name.as_atom())); }, &atom!(placeholder) => { let mut placeholder = self.placeholder.borrow_mut(); @@ -540,6 +539,7 @@ impl VirtualMethods for HTMLInputElement { fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue { match name { + &atom!(name) => AttrValue::from_atomic(value), &atom!("size") => AttrValue::from_limited_u32(value, DEFAULT_INPUT_SIZE), _ => self.super_type().unwrap().parse_plain_attribute(name, value), } @@ -676,7 +676,7 @@ impl Activatable for HTMLInputElement { doc_node.query_selector_iter("input[type=radio]".to_owned()).unwrap() .filter_map(HTMLInputElementCast::to_root) .find(|r| { - in_same_group(r.r(), owner.r(), group.as_ref().map(|gr| &**gr)) && + in_same_group(r.r(), owner.r(), group.as_ref()) && r.r().Checked() }) }; From e72b77b1e3d9c6455094b18bc827f6a8376997eb Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 25 Sep 2015 00:28:13 +0200 Subject: [PATCH 2/2] Properly consider radio button groups Radio button groups with missing or empty names are always the only element of their respective radio button group. --- components/script/dom/htmlinputelement.rs | 13 +++++++++++-- .../wpt/metadata/dom/nodes/Element-matches.html.ini | 6 ------ .../dom/nodes/ParentNode-querySelector-All.html.ini | 12 ------------ 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 8ae1390bf9c..16f640683f8 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -338,6 +338,15 @@ impl HTMLInputElementMethods for HTMLInputElement { #[allow(unsafe_code)] fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) { + match group { + None | Some(&atom!("")) => { + // Radio input elements with a missing or empty name are alone in their + // own group. + return; + }, + _ => {}, + } + //TODO: if not in document, use root ancestor instead of document let owner = broadcaster.form_owner(); let doc = document_from_node(broadcaster); @@ -362,6 +371,7 @@ fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) do_broadcast(doc_node, broadcaster, owner.r(), group) } +// https://html.spec.whatwg.org/multipage/#radio-button-group fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>, group: Option<&Atom>) -> bool { let other_owner = other.form_owner(); @@ -371,8 +381,7 @@ fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>, other_owner == owner && // TODO should be a unicode compatibility caseless match match (other.get_radio_group_name(), group) { - (Some(ref s1), Some(s2)) => s1 == s2, - (None, None) => true, + (Some(ref s1), Some(s2)) => s1 == s2 && s2 != &atom!(""), _ => false } } diff --git a/tests/wpt/metadata/dom/nodes/Element-matches.html.ini b/tests/wpt/metadata/dom/nodes/Element-matches.html.ini index 657462fd41c..d5f32e3c46a 100644 --- a/tests/wpt/metadata/dom/nodes/Element-matches.html.ini +++ b/tests/wpt/metadata/dom/nodes/Element-matches.html.ini @@ -15,9 +15,6 @@ [In-document Element.matches: :lang pseudo-class selector, matching specified language with partial value (with no refNodes): #pseudo-lang-div3:lang(en)] expected: FAIL - [In-document Element.matches: :checked pseudo-class selector, matching checked radio buttons and checkboxes (with no refNodes): #pseudo-ui :checked] - expected: FAIL - [Detached Element.matches: Attribute hyphen-separated list selector, matching lang attribute with partial value (with no refNodes): #attr-hyphen-div3[lang|="en"\]] expected: FAIL @@ -57,9 +54,6 @@ [In-document Element.matches: :lang pseudo-class selector, matching specified language with partial value (1) (with no refNodes): #pseudo-lang-div3:lang(en)] expected: FAIL - [In-document Element.matches: :checked pseudo-class selector, matching checked radio buttons and checkboxes (1) (with no refNodes): #pseudo-ui :checked] - expected: FAIL - [In-document Element.matches: Class selector, matching element with class value using non-ASCII characters (with no refNodes): .台北Táiběi] expected: FAIL diff --git a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini index 15b76b64b57..96469391fe0 100644 --- a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini +++ b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini @@ -48,12 +48,6 @@ [Document.querySelector: :lang pseudo-class selector, not matching incorrect language: #pseudo-lang-div4:lang(es-AR)] expected: FAIL - [Document.querySelectorAll: :checked pseudo-class selector, matching checked radio buttons and checkboxes: #pseudo-ui :checked] - expected: FAIL - - [Document.querySelector: :checked pseudo-class selector, matching checked radio buttons and checkboxes: #pseudo-ui :checked] - expected: FAIL - [Document.querySelectorAll: :first-line pseudo-element (one-colon syntax) selector, not matching any elements: #pseudo-element:first-line] expected: FAIL @@ -252,12 +246,6 @@ [In-document Element.querySelector: :lang pseudo-class selector, not matching incorrect language: #pseudo-lang-div4:lang(es-AR)] expected: FAIL - [In-document Element.querySelectorAll: :checked pseudo-class selector, matching checked radio buttons and checkboxes: #pseudo-ui :checked] - expected: FAIL - - [In-document Element.querySelector: :checked pseudo-class selector, matching checked radio buttons and checkboxes: #pseudo-ui :checked] - expected: FAIL - [In-document Element.querySelectorAll: :first-line pseudo-element (one-colon syntax) selector, not matching any elements: #pseudo-element:first-line] expected: FAIL