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]

Reply via email to