Auto merge of #7733 - nox:checked, r=Manishearth

Properly consider radio button groups

Radio button groups with missing or empty names are always the only element of their respective radio button group.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7733)
<!-- Reviewable:end -->

components/script/dom/htmlinputelement.rs

@@ -296,7 +296,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);
@@ -342,7 +342,16 @@ 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>) {
+    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);
@@ -350,7 +359,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()
@@ -367,8 +376,9 @@ fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&str>)
     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<&str>) -> bool {
+                 group: Option<&Atom>) -> bool {
     let other_owner = other.form_owner();
     let other_owner = other_owner.r();
     other.input_type.get() == InputType::InputRadio &&
@@ -376,8 +386,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 && s2 != &atom!(""),
-        (None, None) => true,
         _ => false
     }
 }
@@ -389,17 +398,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<String> {
+    // https://html.spec.whatwg.org/multipage/#radio-button-group
+    pub fn get_radio_group_name(&self) -> Option<Atom> {
         //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) {
@@ -411,9 +421,7 @@ impl HTMLInputElement {
 
         if self.input_type.get() == InputType::InputRadio && checked {
             broadcast_radio_checked(self,
-                                    self.get_radio_group_name()
+                                    self.get_radio_group_name().as_ref());
-                                        .as_ref()
-                                        .map(|group| &**group));
         }
 
         self.force_relayout();
@@ -509,14 +517,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);
                     }
@@ -529,7 +537,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();
@@ -545,6 +553,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),
         }
@@ -681,7 +690,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()
                                 })
                     };

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
 

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