Don't count <img> elements with both name and id twice in document's named getter (#37455)

A document's named getter collects elements with either matching name or
id's (varies per element type) and returns them .

We implement this the following way:
* Create an iterator with elements whose `name` attribute matches
* Create an iterator with elements whose `id` attribute matches
* Concatenate both

The spec then asks us if there is more than one element in the list,
which we implement by checking whether the iterator returns `None` after
we get the first element. However, the same element can appear in both
iterators if it is a `img` element and both it's name and id attribute
match. Therefore, we need to check if there are more elements *which are
not equal to the first one*.

Testing: New web platform tests pass

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-06-15 20:54:53 +02:00 committed by GitHub
parent ae20cdbdc9
commit 9d10e41a1a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 21 additions and 21 deletions

View file

@ -6126,15 +6126,15 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
self.set_body_attribute(&local_name!("text"), value, can_gc) self.set_body_attribute(&local_name!("text"), value, can_gc)
} }
#[allow(unsafe_code)]
/// <https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter> /// <https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter>
fn NamedGetter(&self, name: DOMString) -> Option<NamedPropertyValue> { fn NamedGetter(&self, name: DOMString, can_gc: CanGc) -> Option<NamedPropertyValue> {
if name.is_empty() { if name.is_empty() {
return None; return None;
} }
let name = Atom::from(name); let name = Atom::from(name);
// Step 1. // Step 1. Let elements be the list of named elements with the name name that are in a document tree
// with the Document as their root.
let elements_with_name = self.get_elements_with_name(&name); let elements_with_name = self.get_elements_with_name(&name);
let name_iter = elements_with_name let name_iter = elements_with_name
.iter() .iter()
@ -6145,10 +6145,14 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
.filter(|elem| is_named_element_with_id_attribute(elem)); .filter(|elem| is_named_element_with_id_attribute(elem));
let mut elements = name_iter.chain(id_iter); let mut elements = name_iter.chain(id_iter);
let first = elements.next()?; // Step 2. If elements has only one element, and that element is an iframe element,
// and that iframe element's content navigable is not null, then return the active
// WindowProxy of the element's content navigable.
if elements.next().is_none() { // NOTE: We have to check if all remaining elements are equal to the first, since
// Step 2. // the same element may appear in both lists.
let first = elements.next()?;
if elements.all(|other| first == other) {
if let Some(nested_window_proxy) = first if let Some(nested_window_proxy) = first
.downcast::<HTMLIFrameElement>() .downcast::<HTMLIFrameElement>()
.and_then(|iframe| iframe.GetContentWindow()) .and_then(|iframe| iframe.GetContentWindow())
@ -6156,11 +6160,12 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
return Some(NamedPropertyValue::WindowProxy(nested_window_proxy)); return Some(NamedPropertyValue::WindowProxy(nested_window_proxy));
} }
// Step 3. // Step 3. Otherwise, if elements has only one element, return that element.
return Some(NamedPropertyValue::Element(DomRoot::from_ref(first))); return Some(NamedPropertyValue::Element(DomRoot::from_ref(first)));
} }
// Step 4. // Step 4. Otherwise, return an HTMLCollection rooted at the Document node,
// whose filter matches only named elements with the name name.
#[derive(JSTraceable, MallocSizeOf)] #[derive(JSTraceable, MallocSizeOf)]
struct DocumentNamedGetter { struct DocumentNamedGetter {
#[no_trace] #[no_trace]
@ -6191,7 +6196,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
self.window(), self.window(),
self.upcast(), self.upcast(),
Box::new(DocumentNamedGetter { name }), Box::new(DocumentNamedGetter { name }),
CanGc::note(), can_gc,
); );
Some(NamedPropertyValue::HTMLCollection(collection)) Some(NamedPropertyValue::HTMLCollection(collection))
} }

View file

@ -122,7 +122,7 @@ impl XMLDocumentMethods<crate::DomTypeHolder> for XMLDocument {
} }
// https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
fn NamedGetter(&self, name: DOMString) -> Option<NamedPropertyValue> { fn NamedGetter(&self, name: DOMString, can_gc: CanGc) -> Option<NamedPropertyValue> {
self.upcast::<Document>().NamedGetter(name) self.upcast::<Document>().NamedGetter(name, can_gc)
} }
} }

View file

@ -167,7 +167,7 @@ DOMInterfaces = {
'Document': { 'Document': {
'additionalTraits': ["crate::interfaces::DocumentHelpers"], 'additionalTraits': ["crate::interfaces::DocumentHelpers"],
'canGc': ['Close', 'CreateElement', 'CreateElementNS', 'ImportNode', 'SetTitle', 'Write', 'Writeln', 'CreateEvent', 'CreateRange', 'Open', 'Open_', 'CreateComment', 'CreateAttribute', 'CreateAttributeNS', 'CreateDocumentFragment', 'CreateTextNode', 'CreateCDATASection', 'CreateProcessingInstruction', 'Prepend', 'Append', 'ReplaceChildren', 'SetBgColor', 'SetFgColor', 'Fonts', 'ElementFromPoint', 'ElementsFromPoint', 'GetScrollingElement', 'ExitFullscreen', 'CreateExpression', 'CreateNSResolver', 'Evaluate', 'StyleSheets', 'Implementation', 'GetElementsByTagName', 'GetElementsByTagNameNS', 'GetElementsByClassName', 'AdoptNode', 'CreateNodeIterator', 'SetBody', 'GetElementsByName', 'Images', 'Embeds', 'Plugins', 'Links', 'Forms', 'Scripts', 'Anchors', 'Applets', 'Children', 'GetSelection'], 'canGc': ['Close', 'CreateElement', 'CreateElementNS', 'ImportNode', 'SetTitle', 'Write', 'Writeln', 'CreateEvent', 'CreateRange', 'Open', 'Open_', 'CreateComment', 'CreateAttribute', 'CreateAttributeNS', 'CreateDocumentFragment', 'CreateTextNode', 'CreateCDATASection', 'CreateProcessingInstruction', 'Prepend', 'Append', 'ReplaceChildren', 'SetBgColor', 'SetFgColor', 'Fonts', 'ElementFromPoint', 'ElementsFromPoint', 'GetScrollingElement', 'ExitFullscreen', 'CreateExpression', 'CreateNSResolver', 'Evaluate', 'StyleSheets', 'Implementation', 'GetElementsByTagName', 'GetElementsByTagNameNS', 'GetElementsByClassName', 'AdoptNode', 'CreateNodeIterator', 'SetBody', 'GetElementsByName', 'Images', 'Embeds', 'Plugins', 'Links', 'Forms', 'Scripts', 'Anchors', 'Applets', 'Children', 'GetSelection', 'NamedGetter'],
}, },
'DissimilarOriginWindow': { 'DissimilarOriginWindow': {
@ -671,6 +671,10 @@ DOMInterfaces = {
'canGc': ['AddModule'], 'canGc': ['AddModule'],
}, },
'XMLDocument': {
'canGc': ['NamedGetter'],
},
'XMLHttpRequest': { 'XMLHttpRequest': {
'canGc': ['Abort', 'GetResponseXML', 'Response', 'Send'], 'canGc': ['Abort', 'GetResponseXML', 'Response', 'Send'],
}, },

View file

@ -1,9 +0,0 @@
[nameditem-01.html]
[img elements that have a name and id attribute with same value.]
expected: FAIL
[Dynamically updating the name attribute from img elements, should be accessible by values.]
expected: FAIL
[Dynamically updating the id attribute from img elements, should be accessible by values.]
expected: FAIL