Closed Bug 1061234 Opened 10 years ago Closed 10 years ago

Element.setAttributeNodeNS() and NamedNodeMap.setNamedItemNS() have wrong behaviour

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Version:
34 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: crimsteam, Assigned: anujagarwal464, Mentored)

References

Details

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140716183446

Steps to reproduce:

When we use Element.setAttributeNodeNS()/NamedNodeMap.setNamedItemNS() and pass attr object which replaces another attr object, then in this returned replaced attr object Firefox unnecessary change prefix and name. Returned attribute shouldn't be modified at all.

Smal test:

<!DOCTYPE html>
<body>

<script>

	var body = document.body;
	
	body.setAttributeNS("www.test1.com", "a:new", "Test1");
	
	var new_attr = document.createAttributeNS("www.test1.com", "b:new");
		
	document.write("New attribute: <br>");
	document.write(new_attr.namespaceURI + " , " + new_attr.prefix + " , " +  new_attr.name + " , " + new_attr.value + "<br><br>");
		
	var attr_after = body.setAttributeNodeNS(new_attr);
	
	document.write("Old attribute after returned: <br>");
	document.write(attr_after .namespaceURI + " , " + attr_after .prefix + " , " +  attr_after .name + " , " + attr_after .value + "<br><br>");
	
	var compare = new_attr == attr_after;
	document.write("new_attr == attr_after: " + compare);

</script>

</body>

And behaviour in browser:
- Firefox and Chrome change prefix (and automaticaly changing Attr.name too)
- IE doesn't change prefix (and Attr.name)

But what is interesting is that when we make reference to attr object before invoking setAttributeNodeNS() Firefox works correct:

<!DOCTYPE html>
<body>

<script>

	var body = document.body;
	
	body.setAttributeNS("www.test1.com", "a:new", "Test1");
	
	var attr_before = body.attributes[0];
	var new_attr = document.createAttributeNS("www.test1.com", "b:new");
	
	document.write("New attribute: <br>");
	document.write(new_attr.namespaceURI + " , " + new_attr.prefix + " , " +  new_attr.name + " , " + new_attr.value + "<br><br>");
	
	document.write("Old attribute before returned: <br>");
	document.write(attr_before.namespaceURI + " , " + attr_before.prefix + " , " +  attr_before.name + " , " + attr_before.value + "<br><br>");
	
	var attr_after = body.setAttributeNodeNS(new_attr);
	
	document.write("Old attribute after returned: <br>");
	document.write(attr_after .namespaceURI + " , " + attr_after .prefix + " , " +  attr_after .name + " , " + attr_after .value + "<br><br>");
	
	var compare = new_attr == attr_after;
	document.write("new_attr == attr_after: " + compare);		

</script>

</body>

And behaviour in browser:
- Chrome change prefix (and automaticaly changing Attr.name too)
- Firefox and IE doesn't change prefix (and Attr.name)

At this moment looks like that only IE has correct behaviour for these two examples. 
It's happend for NamedNodeMap.setNamedItemNS() too, which basically works the same as Element.setAttributeNodeNS().
Summary: Element.setAttributeNodeNS() and Element.setAttributeNode() have wrong behaviour → Element.setAttributeNodeNS() and NamedNodeMap.setNamedItemNS() have wrong behaviour
SetAttributeNodeNS just calls SetNamedItemNS.

The issue seems to be that nsDOMAttributeMap::RemoveAttribute uses the passed-in nodeinfo for the new Attr's nodeinfo instead of using the actual nodeinfo of the attribute the element currently has.  We'd need an equivalent for GetExistingAttrNameFromQName but taking either a namespace/localName pair or a nodeinfo instead of prefix+localname.

Not sure to what extent we care about these APIs, though; I thought we were trying to remove them?
Mentor: bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
Philip observed we cannot really remove them. I'm happy to simplify algorithms though.

The reason we are not keen on removing them is that there is a compatibility risk and to really improve the attribute situation, we would need to get rid of settable values on attributes and that was impossible.

Turning Element.attributes into something simpler was not really feasible either.
Assignee: nobody → nicklebedev37
Hello I'd like to be assigned to this bug please assign me.
I think the changes in bug 1075702 actually fixed this.   Arkadiusz, can you confirm?
Flags: needinfo?(crimsteam)
Confirmed, this bug was fixed in mentioned bug by BZ. Around all attr's bugs what I find or report some time ago probably only this two still exist:
https://bugzilla.mozilla.org/show_bug.cgi?id=706770
https://bugzilla.mozilla.org/show_bug.cgi?id=1055465

List is here:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=27344#c24
Flags: needinfo?(crimsteam)
Witek, bug 1055465 might be a good one to look at.  It'll involve some code auditing to make sure null is never actually returned...
Assignee: nicklebedev37 → anujagarwal464
Status: NEW → RESOLVED
Closed: 10 years ago
Depends on: 1075702
Resolution: --- → FIXED
Boris ok i can take it but if i can resolve this on linux ? and because i am very very fresh in project do you think it's good bug for me ?
You should be able to work on bug 1055465 on Linux, yes.  And I think it's a reasonable first bug...
Great, so could you could assign me to this bug ?
Assuming you mean bug 1055465, done.
You need to log in before you can comment on or make changes to this bug.