599590 - Attrib.value can return empty for case-sensitive attributes after calling removeAttribute

Status: RESOLVED FIXED

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

Description

[T. Brains]

User-Agent:       Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.0; Trident/4.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; .NET4.0C)
Build Identifier: Mozilla/5.0 (Windows NT 6.0; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

This is a regression in the Firefox 4.0 beta, and works fine upto Firefox 3.6. 

This happens if you call removeAttribute with the same attribute name (or removeAttributeNS with the lowercased version), and then apply the case-sensitive version using setAttributeNS and try to get the attribute's value from its Attr node.

Reproducible: Always

Steps to Reproduce:
Use the following code:

	var o=document.documentElement.appendChild(document.createElement("div"));
	o.removeAttribute("","myattrib");
	o.setAttributeNS("","myAttrib","123");
	alert(o.getAttributeNodeNS("","myAttrib").value);
Actual Results:  
Shows an empty alert.

Expected Results:  
Should alert "123".

Comment 1

[T. Brains]

Sorry, in that sample code, the second line should either be:

	o.removeAttribute("myAttrib");

or:

	o.removeAttributeNS("","myAttrib");

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Is the removeAttribute really needed?
Locally that didn't have any effect.

But there is clear some problem..

Comment 3

[T. Brains]

That's just a concise testcase to show the issue.

In my real world situation, I have an element with a lowercase attribute which I want to move to a case-sensitive attribute, so I remove the lowercase version and then assign the real one, and I run into this issue.

I could work around this in various ways, but it feels like there's a real issue here that needs attention.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

There is a real issue yes. Taking

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I think we should fix this before ff4.
I have a patch, but will write some tests tomorrow.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

I wish we get rid of attribute node soon.

This should work reasonable well.
This marks some attribute nodes to be created as "non-namespace-aware", and in
that case they get special handling in html documents.
If attribute node is created using an *NS method, it is always namespace-aware,
so the attribute name handling is case-sensitive even in html documents. 

Doesn't do exactly what webkit does regarding to removing attributes,
but webkit has some strange bugs (having attribute nodes which can't be removed
using removeNamedItem* etc.).

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Hmm, actually, I wouldn't block FF4 for this, but I hope the patch gets reviewed 
and approved.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Given that attribute nodes are getting removed in the webcore spec, do we really want to bother fixing bugs in them?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

This is a regression, so in this case I'd say yes.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 478639 [details] [diff] [review]
patch

rs=me, But if there are any problems with this patch I'd like to just mark this bug WONTFIX. If anyone is using this stuff they better start rewriting their code anyway.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #10)
> If anyone is using this stuff they better start rewriting their
> code anyway.
I mostly agree. The only good reason to use this, IMO, is to
iterate all the attributes.

Comment 12

[Johnny Stenback (:jst)]

Not blocking.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/51cea9dd0d86