Names should now be consistently atoms

components/atoms/static_atoms.txt

@@ -84,6 +84,7 @@ radio
 range
 ratechange
 readystatechange
+referrer
 reftest-wait
 rejectionhandled
 removetrack

components/script/dom/document.rs

@@ -823,6 +823,7 @@ impl Document {
     }
 
     fn get_anchor_by_name(&self, name: &str) -> Option<DomRoot<Element>> {
+        // TODO faster name lookups (see #25548)
         let check_anchor = |node: &HTMLAnchorElement| {
             let elem = node.upcast::<Element>();
             elem.get_attribute(&ns!(), &local_name!("name"))
@@ -4091,9 +4092,7 @@ impl DocumentMethods for Document {
             if element.namespace() != &ns!(html) {
                 return false;
             }
-            element
-                .get_attribute(&ns!(), &local_name!("name"))
-                .map_or(false, |attr| &**attr.value() == &*name)
+            element.get_name().map_or(false, |atom| *atom == *name)
         })
     }
 
@@ -4303,6 +4302,7 @@ impl DocumentMethods for Document {
         }
         // https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
         fn filter_by_name(name: &Atom, node: &Node) -> bool {
+            // TODO faster name lookups (see #25548)
             let html_elem_type = match node.type_id() {
                 NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
                 _ => return false,

components/script/dom/element.rs

@@ -1401,11 +1401,28 @@ impl Element {
     // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
     pub fn get_attribute_by_name(&self, name: DOMString) -> Option<DomRoot<Attr>> {
         let name = &self.parsed_name(name);
-        self.attrs
+        let maybe_attribute = self
+            .attrs
             .borrow()
             .iter()
             .find(|a| a.name() == name)
-            .map(|js| DomRoot::from_ref(&**js))
+            .map(|js| DomRoot::from_ref(&**js));
+
+        fn id_and_name_must_be_atoms(name: &LocalName, maybe_attr: &Option<DomRoot<Attr>>) -> bool {
+            if *name == local_name!("id") || *name == local_name!("name") {
+                match maybe_attr {
+                    None => true,
+                    Some(ref attr) => match *attr.value() {
+                        AttrValue::Atom(_) => true,
+                        _ => false,
+                    },
+                }
+            } else {
+                true
+            }
+        }
+        debug_assert!(id_and_name_must_be_atoms(name, &maybe_attribute));
+        maybe_attribute
     }
 
     pub fn set_attribute_from_parser(
@@ -1800,6 +1817,14 @@ impl Element {
         let other = other.upcast::<Element>();
         self.root_element() == other.root_element()
     }
+
+    pub fn get_id(&self) -> Option<Atom> {
+        self.id_attribute.borrow().clone()
+    }
+
+    pub fn get_name(&self) -> Option<Atom> {
+        self.rare_data().as_ref()?.name_attribute.clone()
+    }
 }
 
 impl ElementMethods for Element {
@@ -1836,6 +1861,8 @@ impl ElementMethods for Element {
     }
 
     // https://dom.spec.whatwg.org/#dom-element-id
+    // This always returns a string; if you'd rather see None
+    // on a null id, call get_id
     fn Id(&self) -> DOMString {
         self.get_string_attribute(&local_name!("id"))
     }
@@ -2783,6 +2810,20 @@ impl VirtualMethods for Element {
                     }
                 }
             },
+            &local_name!("name") => {
+                // Keep the name in rare data for fast access
+                self.ensure_rare_data().name_attribute =
+                    mutation.new_value(attr).and_then(|value| {
+                        let value = value.as_atom();
+                        if value != &atom!("") {
+                            Some(value.clone())
+                        } else {
+                            None
+                        }
+                    });
+                // TODO: notify the document about the name change
+                // once it has a name_map (#25548)
+            },
             _ => {
                 // FIXME(emilio): This is pretty dubious, and should be done in
                 // the relevant super-classes.
@@ -2801,6 +2842,7 @@ impl VirtualMethods for Element {
     fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
         match name {
             &local_name!("id") => AttrValue::from_atomic(value.into()),
+            &local_name!("name") => AttrValue::from_atomic(value.into()),
             &local_name!("class") => AttrValue::from_serialized_tokenlist(value.into()),
             _ => self
                 .super_type()

components/script/dom/htmlanchorelement.rs

@@ -152,7 +152,7 @@ impl HTMLAnchorElementMethods for HTMLAnchorElement {
     make_getter!(Name, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-a-name
-    make_setter!(SetName, "name");
+    make_atomic_setter!(SetName, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-a-rev
     make_getter!(Rev, "rev");

components/script/dom/htmlbuttonelement.rs

@@ -142,7 +142,7 @@ impl HTMLButtonElementMethods for HTMLButtonElement {
     make_getter!(Name, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-fe-name
-    make_setter!(SetName, "name");
+    make_atomic_setter!(SetName, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-button-value
     make_getter!(Value, "value");

components/script/dom/htmlcollection.rs

@@ -366,11 +366,12 @@ impl HTMLCollectionMethods for HTMLCollection {
             return None;
         }
 
+        let key = Atom::from(key);
+
         // Step 2.
         self.elements_iter().find(|elem| {
-            elem.get_string_attribute(&local_name!("id")) == key ||
-                (elem.namespace() == &ns!(html) &&
-                    elem.get_string_attribute(&local_name!("name")) == key)
+            elem.get_id().map_or(false, |id| id == key) ||
+                (elem.namespace() == &ns!(html) && elem.get_name().map_or(false, |id| id == key))
         })
     }
 
@@ -392,17 +393,20 @@ impl HTMLCollectionMethods for HTMLCollection {
         // Step 2
         for elem in self.elements_iter() {
             // Step 2.1
-            let id_attr = elem.get_string_attribute(&local_name!("id"));
-            if !id_attr.is_empty() && !result.contains(&id_attr) {
-                result.push(id_attr)
+            if let Some(id_atom) = elem.get_id() {
+                let id_str = DOMString::from(&*id_atom);
+                if !result.contains(&id_str) {
+                    result.push(id_str);
+                }
             }
             // Step 2.2
-            let name_attr = elem.get_string_attribute(&local_name!("name"));
-            if !name_attr.is_empty() &&
-                !result.contains(&name_attr) &&
-                *elem.namespace() == ns!(html)
-            {
-                result.push(name_attr)
+            if *elem.namespace() == ns!(html) {
+                if let Some(name_atom) = elem.get_name() {
+                    let name_str = DOMString::from(&*name_atom);
+                    if !result.contains(&name_str) {
+                        result.push(name_str)
+                    }
+                }
             }
         }
 

components/script/dom/htmlfieldsetelement.rs

@@ -7,6 +7,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding;
 use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding::HTMLFieldSetElementMethods;
 use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
 use crate::dom::bindings::root::{DomRoot, MutNullableDom};
+use crate::dom::bindings::str::DOMString;
 use crate::dom::document::Document;
 use crate::dom::element::{AttributeMutation, Element};
 use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
@@ -88,6 +89,12 @@ impl HTMLFieldSetElementMethods for HTMLFieldSetElement {
     // https://html.spec.whatwg.org/multipage/#dom-fieldset-disabled
     make_bool_setter!(SetDisabled, "disabled");
 
+    // https://html.spec.whatwg.org/multipage/#dom-fe-name
+    make_atomic_setter!(SetName, "name");
+
+    // https://html.spec.whatwg.org/multipage/#dom-fe-name
+    make_getter!(Name, "name");
+
     // https://html.spec.whatwg.org/multipage/#dom-fae-form
     fn GetForm(&self) -> Option<DomRoot<HTMLFormElement>> {
         self.form_owner()

components/script/dom/htmlformcontrolscollection.rs

@@ -18,6 +18,7 @@ use crate::dom::node::Node;
 use crate::dom::radionodelist::RadioNodeList;
 use crate::dom::window::Window;
 use dom_struct::dom_struct;
+use servo_atoms::Atom;
 
 #[dom_struct]
 pub struct HTMLFormControlsCollection {
@@ -67,9 +68,11 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
             return None;
         }
 
+        let name = Atom::from(name);
+
         let mut filter_map = self.collection.elements_iter().filter_map(|elem| {
-            if elem.get_string_attribute(&local_name!("name")) == name ||
-                elem.get_string_attribute(&local_name!("id")) == name
+            if elem.get_name().map_or(false, |n| n == name) ||
+                elem.get_id().map_or(false, |i| i == name)
             {
                 Some(elem)
             } else {
@@ -90,7 +93,7 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
                 // specifically HTMLFormElement::Elements(),
                 // and the collection filter excludes image inputs.
                 Some(RadioNodeListOrElement::RadioNodeList(
-                    RadioNodeList::new_controls_except_image_inputs(window, &*self.form, name),
+                    RadioNodeList::new_controls_except_image_inputs(window, &*self.form, &name),
                 ))
             }
         // Step 3