From 9d10e41a1a5780139d54ac27b8db88adda72be10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Sun, 15 Jun 2025 20:54:53 +0200 Subject: [PATCH] Don't count elements with both name and id twice in document's named getter (#37455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- components/script/dom/document.rs | 23 +++++++++++-------- components/script/dom/xmldocument.rs | 4 ++-- .../script_bindings/codegen/Bindings.conf | 6 ++++- .../dom-tree-accessors/nameditem-01.html.ini | 9 -------- 4 files changed, 21 insertions(+), 21 deletions(-) delete mode 100644 tests/wpt/meta/html/dom/documents/dom-tree-accessors/nameditem-01.html.ini diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index bf108d6b137..407e53f6dc6 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -6126,15 +6126,15 @@ impl DocumentMethods for Document { self.set_body_attribute(&local_name!("text"), value, can_gc) } - #[allow(unsafe_code)] /// - fn NamedGetter(&self, name: DOMString) -> Option { + fn NamedGetter(&self, name: DOMString, can_gc: CanGc) -> Option { if name.is_empty() { return None; } 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 name_iter = elements_with_name .iter() @@ -6145,10 +6145,14 @@ impl DocumentMethods for Document { .filter(|elem| is_named_element_with_id_attribute(elem)); 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() { - // Step 2. + // NOTE: We have to check if all remaining elements are equal to the first, since + // 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 .downcast::() .and_then(|iframe| iframe.GetContentWindow()) @@ -6156,11 +6160,12 @@ impl DocumentMethods for Document { 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))); } - // 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)] struct DocumentNamedGetter { #[no_trace] @@ -6191,7 +6196,7 @@ impl DocumentMethods for Document { self.window(), self.upcast(), Box::new(DocumentNamedGetter { name }), - CanGc::note(), + can_gc, ); Some(NamedPropertyValue::HTMLCollection(collection)) } diff --git a/components/script/dom/xmldocument.rs b/components/script/dom/xmldocument.rs index 67c20c7369f..8408f0704b7 100644 --- a/components/script/dom/xmldocument.rs +++ b/components/script/dom/xmldocument.rs @@ -122,7 +122,7 @@ impl XMLDocumentMethods for XMLDocument { } // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter - fn NamedGetter(&self, name: DOMString) -> Option { - self.upcast::().NamedGetter(name) + fn NamedGetter(&self, name: DOMString, can_gc: CanGc) -> Option { + self.upcast::().NamedGetter(name, can_gc) } } diff --git a/components/script_bindings/codegen/Bindings.conf b/components/script_bindings/codegen/Bindings.conf index 80f28869d91..535294a462e 100644 --- a/components/script_bindings/codegen/Bindings.conf +++ b/components/script_bindings/codegen/Bindings.conf @@ -167,7 +167,7 @@ DOMInterfaces = { 'Document': { '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': { @@ -671,6 +671,10 @@ DOMInterfaces = { 'canGc': ['AddModule'], }, +'XMLDocument': { + 'canGc': ['NamedGetter'], +}, + 'XMLHttpRequest': { 'canGc': ['Abort', 'GetResponseXML', 'Response', 'Send'], }, diff --git a/tests/wpt/meta/html/dom/documents/dom-tree-accessors/nameditem-01.html.ini b/tests/wpt/meta/html/dom/documents/dom-tree-accessors/nameditem-01.html.ini deleted file mode 100644 index 50950d2751b..00000000000 --- a/tests/wpt/meta/html/dom/documents/dom-tree-accessors/nameditem-01.html.ini +++ /dev/null @@ -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