https://github.com/python/cpython/commit/3735833b18964d656b62ac1f710478e6b111f705
commit: 3735833b18964d656b62ac1f710478e6b111f705
branch: main
author: Gregory P. Smith <[email protected]>
committer: gpshead <[email protected]>
date: 2026-11-01T19:18:21Z
summary:
gh-161110: Make minidom getElementById() stable for duplicate IDs (GH-161111)
Visit child nodes in document order and keep the first cached element
for each ID, so the first element in document order is returned.
ANT-2026-C52DMQ8Z
files:
A Misc/NEWS.d/next/Library/2026-10-22-05-20-11.gh-issue-161110.miKrSQ.rst
M Doc/library/xml.dom.rst
M Lib/test/test_minidom.py
M Lib/xml/dom/minidom.py
diff --git a/Doc/library/xml.dom.rst b/Doc/library/xml.dom.rst
index dbe2de5c447d0cc..e5b5147720802e3 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 446bbe096bd19dc..274c6acab4a6512 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 = ("<doc>"
+ "<e id='y' n='0'/>"
+ "<f><e id='x' n='1'/></f>"
+ "<e id='x' n='2'><e id='x' n='3'/></e>"
+ "<e id='z' n='4'/>"
+ "</doc>")
+ with self.subTest("ATTLIST"):
+ doc = parseString("<!DOCTYPE doc [<!ATTLIST e id ID #IMPLIED>]>" +
+ 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("<!DOCTYPE doc [<!ATTLIST e n CDATA #IMPLIED>]>"
+
+ 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 7cb652a323dcc22..f83cd2898daa3e4 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 000000000000000..e71214d60f1cd1e
--- /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.
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: [email protected]