From 3f5d71048a7cd3895fc20021655731cbb6342dfa Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 05:22:09 +0000 Subject: [PATCH] gh-161110: Make minidom getElementById() stable for duplicate IDs Visit child nodes in document order and keep the first cached element for each ID, so the first element in document order is returned. --- Doc/library/xml.dom.rst | 6 ++++ Lib/test/test_minidom.py | 36 +++++++++++++++++++ Lib/xml/dom/minidom.py | 13 +++---- ...-10-22-05-20-11.gh-issue-161110.miKrSQ.rst | 4 +++ 4 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-10-22-05-20-11.gh-issue-161110.miKrSQ.rst diff --git a/Doc/library/xml.dom.rst b/Doc/library/xml.dom.rst index dbe2de5c447d0c..e5b5147720802e 100644 --- a/Doc/library/xml.dom.rst +++ b/Doc/library/xml.dom.rst @@ -756,6 +756,12 @@ inherits properties from :class:`Node`. Return the element with the given ID, or ``None``. Only attributes declared as being of type ID in the DTD or by :meth:`Element.setIdAttribute` are searched. + If several elements have the same ID, the first one + in document order is returned. + + .. versionchanged:: next + Previously, which of the elements with the same ID was returned + was unspecified and could change between calls. .. method:: Document.getElementsByTagName(tagName) diff --git a/Lib/test/test_minidom.py b/Lib/test/test_minidom.py index 446bbe096bd19d..274c6acab4a651 100644 --- a/Lib/test/test_minidom.py +++ b/Lib/test/test_minidom.py @@ -2143,6 +2143,42 @@ def testSetIdAttributeNode(self): self.confirm(e.isSameNode(doc.getElementById("w")) and a2.isId) + def check_duplicate_id(self, doc): + # The first element in document order is returned, and lookups + # of other IDs which continue the search do not change that. + for i in range(2): + self.assertEqual(doc.getElementById("x").getAttribute("n"), "1") + self.assertEqual(doc.getElementById("y").getAttribute("n"), "0") + self.assertEqual(doc.getElementById("x").getAttribute("n"), "1") + self.assertIsNone(doc.getElementById("missing")) + self.assertEqual(doc.getElementById("x").getAttribute("n"), "1") + self.assertEqual(doc.getElementById("z").getAttribute("n"), "4") + # clear the cache and start the search again + doc.documentElement.setAttribute("a", str(i)) + + def testGetElementByIdDuplicate(self): + body = ("" + "" + "" + "" + "" + "") + with self.subTest("ATTLIST"): + doc = parseString("]>" + + body) + self.check_duplicate_id(doc) + with self.subTest("setIdAttribute"): + doc = parseString(body) + for e in doc.getElementsByTagName("e"): + e.setIdAttribute("id") + self.check_duplicate_id(doc) + with self.subTest("ATTLIST and setIdAttribute"): + doc = parseString("]>" + + body) + for e in doc.getElementsByTagName("e"): + e.setIdAttribute("id") + self.check_duplicate_id(doc) + def assert_recursive_equal(self, doc, doc2): stack = [(doc, doc2)] while stack: diff --git a/Lib/xml/dom/minidom.py b/Lib/xml/dom/minidom.py index 7cb652a323dcc2..f83cd2898daa3e 100644 --- a/Lib/xml/dom/minidom.py +++ b/Lib/xml/dom/minidom.py @@ -2030,8 +2030,9 @@ def getElementById(self, id): result = None while stack: node = stack.pop() - # add child elements to stack for continued searching - stack.extend([child for child in node.childNodes + # add child elements to stack for continued searching, + # last child first so that they are visited in document order + stack.extend([child for child in reversed(node.childNodes) if child.nodeType in _nodeTypes_with_children]) # check this node info = self._get_elem_info(node) @@ -2042,19 +2043,19 @@ def getElementById(self, id): for attr in node.attributes.values(): if attr.namespaceURI: if info.isIdNS(attr.namespaceURI, attr.localName): - self._id_cache[attr.value] = node + self._id_cache.setdefault(attr.value, node) if attr.value == id: result = node elif not node._magic_id_nodes: break elif info.isId(attr.name): - self._id_cache[attr.value] = node + self._id_cache.setdefault(attr.value, node) if attr.value == id: result = node elif not node._magic_id_nodes: break elif attr._is_id: - self._id_cache[attr.value] = node + self._id_cache.setdefault(attr.value, node) if attr.value == id: result = node elif node._magic_id_nodes == 1: @@ -2062,7 +2063,7 @@ def getElementById(self, id): elif node._magic_id_nodes: for attr in node.attributes.values(): if attr._is_id: - self._id_cache[attr.value] = node + self._id_cache.setdefault(attr.value, node) if attr.value == id: result = node if result is not None: diff --git a/Misc/NEWS.d/next/Library/2026-10-22-05-20-11.gh-issue-161110.miKrSQ.rst b/Misc/NEWS.d/next/Library/2026-10-22-05-20-11.gh-issue-161110.miKrSQ.rst new file mode 100644 index 00000000000000..e71214d60f1cd1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-22-05-20-11.gh-issue-161110.miKrSQ.rst @@ -0,0 +1,4 @@ +Fix :meth:`~xml.dom.Document.getElementById` in :mod:`xml.dom.minidom` so +that it returns the first element in document order when several elements +have the same ID. Previously, the result could differ between calls on an +unmodified document.