From a5cb176daf18a0999cb1c537d1562cdb0da23fef Mon Sep 17 00:00:00 2001
From: Giovanni Cappellotto <gcappellotto@fb.com>
Date: Sun, 7 Jul 2019 09:53:06 -0700
Subject: [PATCH] bpo-13127: Fix attr name setter behavior

Update the implementation of attributes `name` setter to correctly
replace the attribute in the owner element.

The update is divided in three main steps:

1. update the implementation of `Attr._set_name` to behave more
   similarly to `Document.renameNode`

2. update the implementation of `Document.renameNode` to use
   `Attr._set_name` if the node is of type "attribute"

3. create a new function `_parseName`, that is used in both
   `Attr._set_name` and `Document.renameNode`, to reduce code
   duplication
---
 Lib/test/test_minidom.py | 10 +++++
 Lib/xml/dom/minidom.py   | 84 ++++++++++++++++++++++------------------
 2 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/Lib/test/test_minidom.py b/Lib/test/test_minidom.py
index 70965854ed1b1c..4a7d56232b2fbf 100644
--- a/Lib/test/test_minidom.py
+++ b/Lib/test/test_minidom.py
@@ -1191,6 +1191,16 @@ def checkRenameNodeSharedConstraints(self, doc, node):
         self.assertRaises(xml.dom.WrongDocumentErr, doc2.renameNode, node,
                           xml.dom.EMPTY_NAMESPACE, "foo")
 
+    def testAttributeNameSetter(self):
+        doc = parseString("<doc a='v'/>")
+        elem = doc.documentElement
+        attrmap = elem.attributes
+        attr = elem.attributes['a']
+
+        attr.name = "b"
+        self.confirm(attr.name == "b" and attr.nodeName == "b")
+        self.assertEqual(doc.toxml(), '<?xml version="1.0" ?><doc b="v"/>')
+
     def testRenameAttribute(self):
         doc = parseString("<doc a='v'/>")
         elem = doc.documentElement
diff --git a/Lib/xml/dom/minidom.py b/Lib/xml/dom/minidom.py
index 464420b76598e0..1bcf000c51fea8 100644
--- a/Lib/xml/dom/minidom.py
+++ b/Lib/xml/dom/minidom.py
@@ -376,10 +376,23 @@ def _get_specified(self):
     def _get_name(self):
         return self._name
 
-    def _set_name(self, value):
-        self._name = value
+    def _set_name(self, name, namespaceURI=EMPTY_NAMESPACE):
+        is_id = self._is_id
+
         if self.ownerElement is not None:
-            _clear_id_cache(self.ownerElement)
+            self.ownerElement.removeAttributeNode(self)
+
+        prefix, localName = _parseName(namespaceURI, name, self.nodeType)
+
+        self._name = name
+        self._prefix = prefix
+        self._localName = localName
+        self.namespaceURI = namespaceURI
+
+        if self.ownerElement is not None:
+            self.ownerElement.setAttributeNode(self)
+            if is_id:
+                self.ownerElement.setIdAttributeNode(self)
 
     nodeName = name = property(_get_name, _set_name)
 
@@ -1806,44 +1819,19 @@ def renameNode(self, n, namespaceURI, name):
         if n.nodeType not in (Node.ELEMENT_NODE, Node.ATTRIBUTE_NODE):
             raise xml.dom.NotSupportedErr(
                 "renameNode() only applies to element and attribute nodes")
-        if namespaceURI != EMPTY_NAMESPACE:
-            if ':' in name:
-                prefix, localName = name.split(':', 1)
-                if (  prefix == "xmlns"
-                      and namespaceURI != xml.dom.XMLNS_NAMESPACE):
-                    raise xml.dom.NamespaceErr(
-                        "illegal use of 'xmlns' prefix")
-            else:
-                if (  name == "xmlns"
-                      and namespaceURI != xml.dom.XMLNS_NAMESPACE
-                      and n.nodeType == Node.ATTRIBUTE_NODE):
-                    raise xml.dom.NamespaceErr(
-                        "illegal use of the 'xmlns' attribute")
-                prefix = None
-                localName = name
-        else:
-            prefix = None
-            localName = None
-        if n.nodeType == Node.ATTRIBUTE_NODE:
-            element = n.ownerElement
-            if element is not None:
-                is_id = n._is_id
-                element.removeAttributeNode(n)
-        else:
-            element = None
-        n.prefix = prefix
-        n._localName = localName
-        n.namespaceURI = namespaceURI
-        n.nodeName = name
+
+        prefix, localName = _parseName(namespaceURI, name, n.nodeType)
+
         if n.nodeType == Node.ELEMENT_NODE:
+            n.prefix = prefix
+            n._localName = localName
+            n.namespaceURI = namespaceURI
+            n.nodeName = name
             n.tagName = name
         else:
             # attribute node
-            n.name = name
-            if element is not None:
-                element.setAttributeNode(n)
-                if is_id:
-                    element.setIdAttributeNode(n)
+            n._set_name(name, namespaceURI)
+
         # It's not clear from a semantic perspective whether we should
         # call the user data handlers for the NODE_RENAMED event since
         # we're re-using the existing node.  The draft spec has been
@@ -1855,6 +1843,28 @@ def renameNode(self, n, namespaceURI, name):
             doc="Top-level element of this document.")
 
 
+def _parseName(namespaceURI, name, nodeType):
+    if namespaceURI == EMPTY_NAMESPACE:
+        return None, None
+
+    if ':' in name:
+        prefix, localName = name.split(':', 1)
+        if (  prefix == "xmlns"
+              and namespaceURI != xml.dom.XMLNS_NAMESPACE):
+            raise xml.dom.NamespaceErr(
+                "illegal use of 'xmlns' prefix")
+    else:
+        if (  name == "xmlns"
+              and namespaceURI != xml.dom.XMLNS_NAMESPACE
+              and nodeType == Node.ATTRIBUTE_NODE):
+            raise xml.dom.NamespaceErr(
+                "illegal use of the 'xmlns' attribute")
+        prefix = None
+        localName = name
+
+    return prefix, localName
+
+
 def _clone_node(node, deep, newOwnerDocument):
     """
     Clone a node and give it the new owner document.