Bug 1075702 - Fixed implementation of Element.setAttributeNode(). r=bz
authorAnuj Agarwal <anujagarwal464@gmail.com>
Thu, 15 Jan 2015 05:12:00 +0100
changeset 253029 acc9dcd07546c13e4ddafe04f36b9069b1ed38e9
parent 253028 797403411392892f0ca64e062bd6543f916a5e84
child 253030 bafcd4598f5d41c4ee76f8722a0f22224e216c03
push id721
push userjlund@mozilla.com
push dateTue, 21 Apr 2015 23:03:33 +0000
treeherdermozilla-release@d27c9211ebb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1075702
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1075702 - Fixed implementation of Element.setAttributeNode(). r=bz
dom/base/Attr.cpp
dom/base/Element.cpp
dom/base/nsDOMAttributeMap.cpp
dom/base/nsDOMAttributeMap.h
dom/base/test/test_bug469304.html
--- a/dom/base/Attr.cpp
+++ b/dom/base/Attr.cpp
@@ -178,17 +178,17 @@ Attr::GetNameAtom(nsIContent* aContent)
   return nameAtom.forget();
 }
 
 NS_IMETHODIMP
 Attr::GetValue(nsAString& aValue)
 {
   Element* element = GetElement();
   if (element) {
-    nsCOMPtr<nsIAtom> nameAtom = GetNameAtom(element);
+    nsCOMPtr<nsIAtom> nameAtom = mNodeInfo->NameAtom();
     element->GetAttr(mNodeInfo->NamespaceID(), nameAtom, aValue);
   }
   else {
     aValue = mValue;
   }
 
   return NS_OK;
 }
@@ -197,17 +197,17 @@ void
 Attr::SetValue(const nsAString& aValue, ErrorResult& aRv)
 {
   Element* element = GetElement();
   if (!element) {
     mValue = aValue;
     return;
   }
 
-  nsCOMPtr<nsIAtom> nameAtom = GetNameAtom(element);
+  nsCOMPtr<nsIAtom> nameAtom = mNodeInfo->NameAtom();
   aRv = element->SetAttr(mNodeInfo->NamespaceID(),
                          nameAtom,
                          mNodeInfo->GetPrefixAtom(),
                          aValue,
                          true);
 }
 
 NS_IMETHODIMP
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1194,17 +1194,19 @@ Element::SetAttributeNode(Attr& aNewAttr
   return Attributes()->SetNamedItem(aNewAttr, aError);
 }
 
 already_AddRefed<Attr>
 Element::RemoveAttributeNode(Attr& aAttribute,
                              ErrorResult& aError)
 {
   OwnerDoc()->WarnOnceAbout(nsIDocument::eRemoveAttributeNode);
-  return Attributes()->RemoveNamedItem(aAttribute.NodeName(), aError);
+  nsAutoString nameSpaceURI;
+  aAttribute.NodeInfo()->GetNamespaceURI(nameSpaceURI);
+  return Attributes()->RemoveNamedItemNS(nameSpaceURI, aAttribute.NodeInfo()->LocalName(), aError);
 }
 
 void
 Element::GetAttributeNS(const nsAString& aNamespaceURI,
                         const nsAString& aLocalName,
                         nsAString& aReturn)
 {
   int32_t nsid =
--- a/dom/base/nsDOMAttributeMap.cpp
+++ b/dom/base/nsDOMAttributeMap.cpp
@@ -311,56 +311,61 @@ nsDOMAttributeMap::SetNamedItemInternal(
     if (aError.Failed()) {
       return nullptr;
     }
 
     NS_ASSERTION(adoptedNode == &aAttr, "Uh, adopt node changed nodes?");
   }
 
   // Get nodeinfo and preexisting attribute (if it exists)
-  nsAutoString name;
-  nsRefPtr<mozilla::dom::NodeInfo> ni;
+  nsRefPtr<NodeInfo> oldNi;
+
+  if (!aWithNS) {
+    nsAutoString name;
+    aAttr.GetName(name);
+    oldNi = mContent->GetExistingAttrNameFromQName(name);
+  }
+  else {
+    uint32_t i, count = mContent->GetAttrCount();
+    for (i = 0; i < count; ++i) {
+      const nsAttrName* name = mContent->GetAttrNameAt(i);
+      int32_t attrNS = name->NamespaceID();
+      nsIAtom* nameAtom = name->LocalName();
+
+      // we're purposefully ignoring the prefix.
+      if (aAttr.NodeInfo()->Equals(nameAtom, attrNS)) {
+        oldNi = mContent->NodeInfo()->NodeInfoManager()->
+          GetNodeInfo(nameAtom, name->GetPrefix(), aAttr.NodeInfo()->NamespaceID(),
+                      nsIDOMNode::ATTRIBUTE_NODE);
+        break;
+      }
+    }
+  }
 
   nsRefPtr<Attr> attr;
-  // SetNamedItemNS()
-  if (aWithNS) {
-    // Return existing attribute, if present
-    ni = aAttr.NodeInfo();
 
-    if (mContent->HasAttr(ni->NamespaceID(), ni->NameAtom())) {
-      attr = RemoveAttribute(ni);
-    }
-  } else { // SetNamedItem()
-    aAttr.GetName(name);
+  if (oldNi) {
+    nsRefPtr<Attr> oldAttr = GetAttribute(oldNi, true);
 
-    // get node-info of old attribute
-    ni = mContent->GetExistingAttrNameFromQName(name);
-    if (ni) {
-      attr = RemoveAttribute(ni);
+    if (oldAttr == &aAttr) {
+      return oldAttr.forget();
     }
-    else {
-      if (mContent->IsInHTMLDocument() &&
-          mContent->IsHTML()) {
-        nsContentUtils::ASCIIToLower(name);
-      }
 
-      rv = mContent->NodeInfo()->NodeInfoManager()->
-        GetNodeInfo(name, nullptr, kNameSpaceID_None,
-                    nsIDOMNode::ATTRIBUTE_NODE, getter_AddRefs(ni));
-      if (NS_FAILED(rv)) {
-        aError.Throw(rv);
-        return nullptr;
-      }
-      // value is already empty
+    if (oldAttr) {
+      attr = RemoveNamedItem(oldNi, aError);
+      NS_ASSERTION(attr->NodeInfo()->NameAndNamespaceEquals(oldNi),
+        "RemoveNamedItem() called, attr->NodeInfo() should be equal to oldNi!");
     }
   }
 
   nsAutoString value;
   aAttr.GetValue(value);
 
+  nsRefPtr<NodeInfo> ni = aAttr.NodeInfo();
+
   // Add the new attribute to the attribute map before updating
   // its value in the element. @see bug 364413.
   nsAttrKey attrkey(ni->NamespaceID(), ni->NameAtom());
   EnsureAttributeCache();
   mAttributeCache->Put(attrkey, &aAttr);
   aAttr.SetMap(this);
 
   rv = mContent->SetAttr(ni->NamespaceID(), ni->NameAtom(),
@@ -368,16 +373,25 @@ nsDOMAttributeMap::SetNamedItemInternal(
   if (NS_FAILED(rv)) {
     aError.Throw(rv);
     DropAttribute(ni->NamespaceID(), ni->NameAtom());
   }
 
   return attr.forget();
 }
 
+already_AddRefed<Attr>
+nsDOMAttributeMap::RemoveNamedItem(NodeInfo* aNodeInfo, ErrorResult& aError)
+{
+  nsRefPtr<Attr> attribute = GetAttribute(aNodeInfo, true);
+  // This removes the attribute node from the attribute map.
+  aError = mContent->UnsetAttr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom(), true);
+  return attribute.forget();
+}
+
 NS_IMETHODIMP
 nsDOMAttributeMap::RemoveNamedItem(const nsAString& aName,
                                    nsIDOMAttr** aReturn)
 {
   NS_ENSURE_ARG_POINTER(aReturn);
 
   ErrorResult rv;
   *aReturn = RemoveNamedItem(aName, rv).take();
@@ -393,21 +407,17 @@ nsDOMAttributeMap::RemoveNamedItem(const
   }
 
   nsRefPtr<mozilla::dom::NodeInfo> ni = mContent->GetExistingAttrNameFromQName(aName);
   if (!ni) {
     aError.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
     return nullptr;
   }
 
-  nsRefPtr<Attr> attribute = GetAttribute(ni, true);
-
-  // This removes the attribute node from the attribute map.
-  aError = mContent->UnsetAttr(ni->NamespaceID(), ni->NameAtom(), true);
-  return attribute.forget();
+  return RemoveNamedItem(ni, aError);
 }
 
 
 Attr*
 nsDOMAttributeMap::IndexedGetter(uint32_t aIndex, bool& aFound)
 {
   aFound = false;
   NS_ENSURE_TRUE(mContent, nullptr);
@@ -495,16 +505,17 @@ nsDOMAttributeMap::GetAttrNodeInfo(const
   }
 
   uint32_t i, count = mContent->GetAttrCount();
   for (i = 0; i < count; ++i) {
     const nsAttrName* name = mContent->GetAttrNameAt(i);
     int32_t attrNS = name->NamespaceID();
     nsIAtom* nameAtom = name->LocalName();
 
+    // we're purposefully ignoring the prefix.
     if (nameSpaceID == attrNS &&
         nameAtom->Equals(aLocalName)) {
       nsRefPtr<mozilla::dom::NodeInfo> ni;
       ni = mContent->NodeInfo()->NodeInfoManager()->
         GetNodeInfo(nameAtom, name->GetPrefix(), nameSpaceID,
                     nsIDOMNode::ATTRIBUTE_NODE);
 
       return ni.forget();
@@ -531,21 +542,17 @@ nsDOMAttributeMap::RemoveNamedItemNS(con
                                      ErrorResult& aError)
 {
   nsRefPtr<mozilla::dom::NodeInfo> ni = GetAttrNodeInfo(aNamespaceURI, aLocalName);
   if (!ni) {
     aError.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
     return nullptr;
   }
 
-  nsRefPtr<Attr> attr = RemoveAttribute(ni);
-  mozilla::dom::NodeInfo* attrNi = attr->NodeInfo();
-  mContent->UnsetAttr(attrNi->NamespaceID(), attrNi->NameAtom(), true);
-
-  return attr.forget();
+  return RemoveNamedItem(ni, aError);
 }
 
 uint32_t
 nsDOMAttributeMap::Count() const
 {
   return mAttributeCache ? mAttributeCache->Count() : 0;
 }
 
--- a/dom/base/nsDOMAttributeMap.h
+++ b/dom/base/nsDOMAttributeMap.h
@@ -147,16 +147,18 @@ public:
   Attr* NamedGetter(const nsAString& aAttrName, bool& aFound);
   bool NameIsEnumerable(const nsAString& aName);
   already_AddRefed<Attr>
   SetNamedItem(Attr& aAttr, ErrorResult& aError)
   {
     return SetNamedItemInternal(aAttr, false, aError);
   }
   already_AddRefed<Attr>
+  RemoveNamedItem(mozilla::dom::NodeInfo* aNodeInfo, ErrorResult& aError);
+  already_AddRefed<Attr>
   RemoveNamedItem(const nsAString& aName, ErrorResult& aError);
  
   Attr* Item(uint32_t aIndex);
   Attr* IndexedGetter(uint32_t aIndex, bool& aFound);
   uint32_t Length() const;
 
   Attr*
   GetNamedItemNS(const nsAString& aNamespaceURI,
--- a/dom/base/test/test_bug469304.html
+++ b/dom/base/test/test_bug469304.html
@@ -22,47 +22,47 @@ https://bugzilla.mozilla.org/show_bug.cg
 function testGetAttribute() {
   var a1 = document.createAttribute("aa");
   a1.nodeValue = "lowercase";
   var a2 = document.createAttribute("AA");
   a2.nodeValue = "UPPERCASE";
   document.body.setAttributeNode(a1);
   document.body.setAttributeNode(a2);
   var log = document.getElementById("log");
-  is(document.body.getAttribute('aa'), "UPPERCASE", "Wrong value (1)");
-  is(document.body.getAttribute('AA'), "UPPERCASE", "Wrong value (2)");
+  is(document.body.getAttribute('aa'), null, "Attribute has the localName AA and not aa.");
+  is(document.body.getAttribute('AA'), null, "Attribute has the localName AA and not aa.");
+  is(document.body.getAttributeNS("", "aa"), null, "Attribute should have localName AA.");
+  is(document.body.getAttributeNS("", "AA"), "UPPERCASE", "Attribute should have value UPPERCASE!");
 
   var s = "";
   for (var i = 0; i < document.body.attributes.length; ++i) {
     s += document.body.attributes[i].nodeName + "=" +
          document.body.attributes[i].nodeValue;
   }
   is(s, "AA=UPPERCASE", "Wrong attribute!");
 
   is(document.body.getAttributeNode("aa"), document.body.getAttributeNode("AA"),
      "Wrong node!");
 
-  document.body.getAttributeNode("AA").nodeValue = "FOO";
-  is(document.body.getAttribute("AA"), "FOO", "Wrong value!");
+  document.body.getAttributeNodeNS("", "AA").nodeValue = "FOO";
+  is(document.body.getAttributeNS("", "AA"), "FOO", "Wrong value!");
 
-  document.body.removeAttributeNode(document.body.getAttributeNode("AA"));
-  ok(!document.body.hasAttribute("AA"), "Should not have attribute!");
+  document.body.removeAttributeNode(document.body.getAttributeNodeNS("", "AA"));
   ok(!document.body.getAttributeNode("AA"), "Should not have attribute node!");
-  ok(!document.body.hasAttribute("aa"), "Should not have attribute!");
   ok(!document.body.getAttributeNode("aa"), "Should not have attribute node!");
 
   is(a2.nodeValue, "FOO", "Wrong value!");
   a2.nodeValue = "UPPERCASE";
   is(a2.nodeValue, "UPPERCASE", "Wrong value!");
 
   document.body.setAttributeNode(a2);
-  is(document.body.getAttribute("AA"), "UPPERCASE", "wrong value!");
-  ok(document.body.getAttributeNode("AA"), "Should have attribute node!");
-  is(document.body.getAttribute("aa"), "UPPERCASE", "wrong value!");
-  ok(document.body.getAttributeNode("aa"), "Should have attribute node!");
+  is(document.body.getAttributeNS("", "AA"), "UPPERCASE", "Wrong value!");
+  ok(document.body.getAttributeNodeNS("", "AA"), "Should have attribute node!");
+  is(document.body.getAttributeNS("", "aa"), null, "No attribute has the localName aa.");
+  ok(!document.body.getAttributeNodeNS("", "aa"), "Should not have attribute node!");
 }
 testGetAttribute();
 
 // A bit modified testcases from WebKit.
 function testGetAttributeCaseInsensitive() {
   var div = document.createElement('div');
   div.setAttribute("mixedCaseAttrib", "x");
   // Do original case lookup, and lowercase lookup.
@@ -70,17 +70,17 @@ function testGetAttributeCaseInsensitive
 }
 is(testGetAttributeCaseInsensitive(), "x", "(1)");
 
 function testGetAttributeNodeMixedCase() {
   var div = document.createElement('div');
   var a = div.ownerDocument.createAttribute("mixedCaseAttrib");
   a.nodeValue = "x";
   div.setAttributeNode(a);
-  return div.getAttribute("mixedCaseAttrib");
+  return div.getAttributeNS("", "mixedCaseAttrib");
 }
 is(testGetAttributeNodeMixedCase(), "x", "(2)");
 
 function testGetAttributeNodeLowerCase(div) {
   var div = document.createElement('div');
   var a = div.ownerDocument.createAttribute("lowercaseattrib");
   a.nodeValue = "x";
   div.setAttributeNode(a);
@@ -109,37 +109,37 @@ function testAttribNodeNamePreservesCase
 }
 is(testAttribNodeNamePreservesCase(), "A,A", "(5)");
 
 function testAttribNodeNamePreservesCaseGetNode() {
   var body = document.body;
   var a = body.ownerDocument.createAttribute("A");
   a.nodeValue = "x";
   body.setAttributeNode(a);
-  a = document.body.getAttributeNode("A");
+  a = document.body.getAttributeNodeNS("", "A");
   if (!a)
       return "FAIL";
   var result = [ a.name, a.nodeName ];
   return result.join(",");
 }
 is(testAttribNodeNamePreservesCaseGetNode(), "A,A", "(6)");
 
 function testAttribNodeNamePreservesCaseGetNode2() {
   var body = document.body;
   var a = body.ownerDocument.createAttribute("B");
   a.nodeValue = "x";
   body.setAttributeNode(a);
-  a = document.body.getAttributeNode("B");
+  a = document.body.getAttributeNodeNS("", "B");
   if (!a)
       return "FAIL";
   // Now create node second time
   a = body.ownerDocument.createAttribute("B");
   a.nodeValue = "x";
   body.setAttributeNode(a);
-  a = document.body.getAttributeNode("B");
+  a = document.body.getAttributeNodeNS("", "B");
   var result = [ a.name, a.nodeName ];
   return result.join(",");
 }
 is(testAttribNodeNamePreservesCaseGetNode2(), "B,B", "(7)");
 
 function testAttribNodeNameGetMutate() {
   var body = document.body;
   var a = body.ownerDocument.createAttribute("c");
@@ -153,19 +153,19 @@ function testAttribNodeNameGetMutate() {
 is(testAttribNodeNameGetMutate(), "1", "(8)");
 
 var node = document.createElement("div");
 var attrib = document.createAttribute("myAttrib");
 attrib.nodeValue = "XXX";
 node.setAttributeNode(attrib);
 // Note, this is different to what WebKit does
 is((new XMLSerializer).serializeToString(node),
-   "<div xmlns=\"http://www.w3.org/1999/xhtml\" myattrib=\"XXX\"></div>", "(9)");
-is(node.getAttributeNode('myAttrib').name, "myAttrib", "(10)");
-is(node.getAttributeNode('myattrib').name, "myAttrib", "(11)");
+   "<div xmlns=\"http://www.w3.org/1999/xhtml\" myAttrib=\"XXX\"></div>", "(9)");
+is(node.getAttributeNodeNS("", "myAttrib").name, "myAttrib", "(10)");
+is(node.getAttributeNodeNS("", "myattrib"), null, "(11)");
 is(attrib.name, "myAttrib", "(12)");
 
 var o = document.createElement("div");
 o.setAttribute("myAttrib","htmlattr");
 o.setAttributeNS("","myAttrib","123");
 is(o.getAttributeNodeNS("","myAttrib").nodeName, "myAttrib", "nodeName should be case-sensitive.");
 is(o.getAttributeNode("myAttrib").nodeName, "myattrib", "nodeName shouldn't be case-sensitive.");
 is(o.getAttributeNodeNS("","myAttrib").value, "123", "Should have got the case-sensitive attribute.");