Closed
Bug 599590
Opened 14 years ago
Closed 14 years ago
Attrib.value can return empty for case-sensitive attributes after calling removeAttribute
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: t.brain, Assigned: smaug)
References
Details
Attachments
(1 file)
12.24 KB,
patch
|
sicking
:
review+
jst
:
approval2.0+
|
Details | Diff | Splinter Review |
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".
Sorry, in that sample code, the second line should either be: o.removeAttribute("myAttrib"); or: o.removeAttributeNS("","myAttrib");
Assignee | ||
Comment 2•14 years ago
|
||
Is the removeAttribute really needed? Locally that didn't have any effect. But there is clear some problem..
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.
Assignee | ||
Comment 4•14 years ago
|
||
There is a real issue yes. Taking
Assignee: nobody → Olli.Pettay
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 5•14 years ago
|
||
I think we should fix this before ff4. I have a patch, but will write some tests tomorrow.
blocking2.0: --- → ?
Assignee | ||
Comment 6•14 years ago
|
||
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.).
Attachment #478639 -
Flags: review?(jonas)
Assignee | ||
Comment 7•14 years ago
|
||
Hmm, actually, I wouldn't block FF4 for this, but I hope the patch gets reviewed and approved.
Given that attribute nodes are getting removed in the webcore spec, do we really want to bother fixing bugs in them?
Assignee | ||
Comment 9•14 years ago
|
||
This is a regression, so in this case I'd say yes.
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.
Attachment #478639 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 11•14 years ago
|
||
(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.
Assignee | ||
Updated•14 years ago
|
Attachment #478639 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #478639 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 13•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/51cea9dd0d86
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•