Skip to content

Commit f29e22f

Browse files
pshaughnpshaughn
committed
Names should now be consistently atoms
1 parent 43c558f commit f29e22f

32 files changed

+270
-652
lines changed

‎components/atoms/static_atoms.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ radio
8484
range
8585
ratechange
8686
readystatechange
87+
referrer
8788
reftest-wait
8889
rejectionhandled
8990
removetrack

‎components/script/dom/document.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,7 @@ impl Document {
823823
}
824824

825825
fn get_anchor_by_name(&self, name: &str) -> Option<DomRoot<Element>> {
826+
// TODO faster name lookups (see #25548)
826827
let check_anchor = |node: &HTMLAnchorElement| {
827828
let elem = node.upcast::<Element>();
828829
elem.get_attribute(&ns!(), &local_name!("name"))
@@ -4091,9 +4092,7 @@ impl DocumentMethods for Document {
40914092
if element.namespace() != &ns!(html) {
40924093
return false;
40934094
}
4094-
element
4095-
.get_attribute(&ns!(), &local_name!("name"))
4096-
.map_or(false, |attr| &**attr.value() == &*name)
4095+
element.get_name().map_or(false, |atom| *atom == *name)
40974096
})
40984097
}
40994098

@@ -4303,6 +4302,7 @@ impl DocumentMethods for Document {
43034302
}
43044303
// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
43054304
fn filter_by_name(name: &Atom, node: &Node) -> bool {
4305+
// TODO faster name lookups (see #25548)
43064306
let html_elem_type = match node.type_id() {
43074307
NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
43084308
_ => return false,

‎components/script/dom/element.rs‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,11 +1401,28 @@ impl Element {
14011401
// https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
14021402
pub fn get_attribute_by_name(&self, name: DOMString) -> Option<DomRoot<Attr>> {
14031403
let name = &self.parsed_name(name);
1404-
self.attrs
1404+
let maybe_attribute = self
1405+
.attrs
14051406
.borrow()
14061407
.iter()
14071408
.find(|a| a.name() == name)
1408-
.map(|js| DomRoot::from_ref(&**js))
1409+
.map(|js| DomRoot::from_ref(&**js));
1410+
1411+
fn id_and_name_must_be_atoms(name: &LocalName, maybe_attr: &Option<DomRoot<Attr>>) -> bool {
1412+
if *name == local_name!("id") || *name == local_name!("name") {
1413+
match maybe_attr {
1414+
None => true,
1415+
Some(ref attr) => match *attr.value() {
1416+
AttrValue::Atom(_) => true,
1417+
_ => false,
1418+
},
1419+
}
1420+
} else {
1421+
true
1422+
}
1423+
}
1424+
debug_assert!(id_and_name_must_be_atoms(name, &maybe_attribute));
1425+
maybe_attribute
14091426
}
14101427

14111428
pub fn set_attribute_from_parser(
@@ -1800,6 +1817,14 @@ impl Element {
18001817
let other = other.upcast::<Element>();
18011818
self.root_element() == other.root_element()
18021819
}
1820+
1821+
pub fn get_id(&self) -> Option<Atom> {
1822+
self.id_attribute.borrow().clone()
1823+
}
1824+
1825+
pub fn get_name(&self) -> Option<Atom> {
1826+
self.rare_data().as_ref()?.name_attribute.clone()
1827+
}
18031828
}
18041829

18051830
impl ElementMethods for Element {
@@ -1836,6 +1861,8 @@ impl ElementMethods for Element {
18361861
}
18371862

18381863
// https://dom.spec.whatwg.org/#dom-element-id
1864+
// This always returns a string; if you'd rather see None
1865+
// on a null id, call get_id
18391866
fn Id(&self) -> DOMString {
18401867
self.get_string_attribute(&local_name!("id"))
18411868
}
@@ -2783,6 +2810,20 @@ impl VirtualMethods for Element {
27832810
}
27842811
}
27852812
},
2813+
&local_name!("name") => {
2814+
// Keep the name in rare data for fast access
2815+
self.ensure_rare_data().name_attribute =
2816+
mutation.new_value(attr).and_then(|value| {
2817+
let value = value.as_atom();
2818+
if value != &atom!("") {
2819+
Some(value.clone())
2820+
} else {
2821+
None
2822+
}
2823+
});
2824+
// TODO: notify the document about the name change
2825+
// once it has a name_map (#25548)
2826+
},
27862827
_ => {
27872828
// FIXME(emilio): This is pretty dubious, and should be done in
27882829
// the relevant super-classes.
@@ -2801,6 +2842,7 @@ impl VirtualMethods for Element {
28012842
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
28022843
match name {
28032844
&local_name!("id") => AttrValue::from_atomic(value.into()),
2845+
&local_name!("name") => AttrValue::from_atomic(value.into()),
28042846
&local_name!("class") => AttrValue::from_serialized_tokenlist(value.into()),
28052847
_ => self
28062848
.super_type()

‎components/script/dom/htmlanchorelement.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ impl HTMLAnchorElementMethods for HTMLAnchorElement {
152152
make_getter!(Name, "name");
153153

154154
// https://html.spec.whatwg.org/multipage/#dom-a-name
155-
make_setter!(SetName, "name");
155+
make_atomic_setter!(SetName, "name");
156156

157157
// https://html.spec.whatwg.org/multipage/#dom-a-rev
158158
make_getter!(Rev, "rev");

‎components/script/dom/htmlbuttonelement.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ impl HTMLButtonElementMethods for HTMLButtonElement {
142142
make_getter!(Name, "name");
143143

144144
// https://html.spec.whatwg.org/multipage/#dom-fe-name
145-
make_setter!(SetName, "name");
145+
make_atomic_setter!(SetName, "name");
146146

147147
// https://html.spec.whatwg.org/multipage/#dom-button-value
148148
make_getter!(Value, "value");

‎components/script/dom/htmlcollection.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -366,11 +366,12 @@ impl HTMLCollectionMethods for HTMLCollection {
366366
return None;
367367
}
368368

369+
let key = Atom::from(key);
370+
369371
// Step 2.
370372
self.elements_iter().find(|elem| {
371-
elem.get_string_attribute(&local_name!("id")) == key ||
372-
(elem.namespace() == &ns!(html) &&
373-
elem.get_string_attribute(&local_name!("name")) == key)
373+
elem.get_id().map_or(false, |id| id == key) ||
374+
(elem.namespace() == &ns!(html) && elem.get_name().map_or(false, |id| id == key))
374375
})
375376
}
376377

@@ -392,17 +393,20 @@ impl HTMLCollectionMethods for HTMLCollection {
392393
// Step 2
393394
for elem in self.elements_iter() {
394395
// Step 2.1
395-
let id_attr = elem.get_string_attribute(&local_name!("id"));
396-
if !id_attr.is_empty() && !result.contains(&id_attr) {
397-
result.push(id_attr)
396+
if let Some(id_atom) = elem.get_id() {
397+
let id_str = DOMString::from(&*id_atom);
398+
if !result.contains(&id_str) {
399+
result.push(id_str);
400+
}
398401
}
399402
// Step 2.2
400-
let name_attr = elem.get_string_attribute(&local_name!("name"));
401-
if !name_attr.is_empty() &&
402-
!result.contains(&name_attr) &&
403-
*elem.namespace() == ns!(html)
404-
{
405-
result.push(name_attr)
403+
if *elem.namespace() == ns!(html) {
404+
if let Some(name_atom) = elem.get_name() {
405+
let name_str = DOMString::from(&*name_atom);
406+
if !result.contains(&name_str) {
407+
result.push(name_str)
408+
}
409+
}
406410
}
407411
}
408412

‎components/script/dom/htmlfieldsetelement.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding;
77
use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding::HTMLFieldSetElementMethods;
88
use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
99
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
10+
use crate::dom::bindings::str::DOMString;
1011
use crate::dom::document::Document;
1112
use crate::dom::element::{AttributeMutation, Element};
1213
use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
@@ -88,6 +89,12 @@ impl HTMLFieldSetElementMethods for HTMLFieldSetElement {
8889
// https://html.spec.whatwg.org/multipage/#dom-fieldset-disabled
8990
make_bool_setter!(SetDisabled, "disabled");
9091

92+
// https://html.spec.whatwg.org/multipage/#dom-fe-name
93+
make_atomic_setter!(SetName, "name");
94+
95+
// https://html.spec.whatwg.org/multipage/#dom-fe-name
96+
make_getter!(Name, "name");
97+
9198
// https://html.spec.whatwg.org/multipage/#dom-fae-form
9299
fn GetForm(&self) -> Option<DomRoot<HTMLFormElement>> {
93100
self.form_owner()

‎components/script/dom/htmlformcontrolscollection.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::dom::node::Node;
1818
use crate::dom::radionodelist::RadioNodeList;
1919
use crate::dom::window::Window;
2020
use dom_struct::dom_struct;
21+
use servo_atoms::Atom;
2122

2223
#[dom_struct]
2324
pub struct HTMLFormControlsCollection {
@@ -67,9 +68,11 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
6768
return None;
6869
}
6970

71+
let name = Atom::from(name);
72+
7073
let mut filter_map = self.collection.elements_iter().filter_map(|elem| {
71-
if elem.get_string_attribute(&local_name!("name")) == name ||
72-
elem.get_string_attribute(&local_name!("id")) == name
74+
if elem.get_name().map_or(false, |n| n == name) ||
75+
elem.get_id().map_or(false, |i| i == name)
7376
{
7477
Some(elem)
7578
} else {
@@ -90,7 +93,7 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
9093
// specifically HTMLFormElement::Elements(),
9194
// and the collection filter excludes image inputs.
9295
Some(RadioNodeListOrElement::RadioNodeList(
93-
RadioNodeList::new_controls_except_image_inputs(window, &*self.form, name),
96+
RadioNodeList::new_controls_except_image_inputs(window, &*self.form, &name),
9497
))
9598
}
9699
// Step 3

0 commit comments

Comments
 (0)