Skip to content

Commit

Permalink
Bug 1075702 - Fixed implementation of Element.setAttributeNode().
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmottola
rmottola committed Feb 16, 2019
1 parent da6beb8 commit 8e5cdb9
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 59 deletions.
4 changes: 2 additions & 2 deletions dom/base/Attr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ 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 {
Expand All @@ -202,7 +202,7 @@ Attr::SetValue(const nsAString& aValue, ErrorResult& aRv)
return;
}

nsCOMPtr<nsIAtom> nameAtom = GetNameAtom(element);
nsCOMPtr<nsIAtom> nameAtom = mNodeInfo->NameAtom();
aRv = element->SetAttr(mNodeInfo->NamespaceID(),
nameAtom,
mNodeInfo->GetPrefixAtom(),
Expand Down
4 changes: 3 additions & 1 deletion dom/base/Element.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,9 @@ Element::RemoveAttributeNode(Attr& aAttribute,
}

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
Expand Down
83 changes: 45 additions & 38 deletions dom/base/nsDOMAttributeMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,46 +317,51 @@ nsDOMAttributeMap::SetNamedItemInternal(Attr& aAttr,
}

// 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());
Expand All @@ -374,6 +379,15 @@ nsDOMAttributeMap::SetNamedItemInternal(Attr& aAttr,
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)
Expand All @@ -399,11 +413,7 @@ nsDOMAttributeMap::RemoveNamedItem(const nsAString& aName, ErrorResult& aError)
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);
}


Expand Down Expand Up @@ -501,6 +511,7 @@ nsDOMAttributeMap::GetAttrNodeInfo(const nsAString& aNamespaceURI,
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;
Expand Down Expand Up @@ -537,11 +548,7 @@ nsDOMAttributeMap::RemoveNamedItemNS(const nsAString& aNamespaceURI,
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
Expand Down
2 changes: 2 additions & 0 deletions dom/base/nsDOMAttributeMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ class nsDOMAttributeMap : public nsIDOMMozNamedAttrMap
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);
Expand Down
36 changes: 18 additions & 18 deletions dom/base/test/test_bug469304.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@
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) {
Expand All @@ -40,24 +42,22 @@
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();

Expand All @@ -75,7 +75,7 @@
var a = div.ownerDocument.createAttribute("mixedCaseAttrib");
a.nodeValue = "x";
div.setAttributeNode(a);
return div.getAttribute("mixedCaseAttrib");
return div.getAttributeNS("", "mixedCaseAttrib");
}
is(testGetAttributeNodeMixedCase(), "x", "(2)");

Expand Down Expand Up @@ -114,7 +114,7 @@
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 ];
Expand All @@ -127,14 +127,14 @@
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(",");
}
Expand All @@ -158,9 +158,9 @@
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");
Expand Down

0 comments on commit 8e5cdb9

Please sign in to comment.