Ticket #12072 (closed bug: fixed)
Remove Firefox deprecated nodeValue, getAttributeNode, specified
| Reported by: | dmethvin | Owned by: | timmywil |
|---|---|---|---|
| Priority: | high | Milestone: | 1.10/2.0 |
| Component: | attributes | Version: | 1.7.2 |
| Keywords: | Cc: | ||
| Blocking: | Blocked by: |
Description
Firefox 13 generates a pretty large set of warnings in our attribute unit tests:
Use of getAttributeNode() is deprecated. Use getAttribute() instead. Use of attributes' nodeValue attribute is deprecated. Use value instead. Use of document.createAttribute() is deprecated. Use element.setAttribute() instead. Use of attributes' specified attribute is deprecated. It always returns true.
The fact that .specified is already broken may already be causing some bugs. We should either 1) remove these uses entirely or 2) feature test them so we don't use them in browsers where they're unsupported.
Ideally we'd have a way to do that feature testing without making Firefox blurt something out on the console. People are really sensitive about that, and no amount of reasoning will make them understand that a warning does not equate to an error. See #4774, #7291, #7535, and #10531 for example.
Change History
comment:1 Changed 10 months ago by dmethvin
- Priority changed from undecided to high
- Status changed from new to open
- Component changed from unfiled to attributes
- Milestone changed from None to 1.9
comment:4 Changed 10 months ago by dmethvin
- Summary changed from Remove/avoid deprecated attribute features to Remove Firefox deprecated nodeValue, getAttributeNode, specified
comment:5 Changed 10 months ago by timmywil
The only codepath in 1.8 that I can find that uses getAttributeNode in Firefox is the propHook for tabIndex. That could use a feature detect.
comment:6 Changed 10 months ago by timmywil
Also, the option valHook uses the attributes property, which is sort of the feature detect itself. However, even with the warning and specified always being true, that valHook will continue working. We could still adjust it if we want.
comment:7 Changed 10 months ago by timmywil
Btw, Neither event.js nor native Sizzle uses the attributes property in FF anymore.
comment:8 Changed 10 months ago by dmethvin
We're using .specified in Sizzle.attr, which according to the Firefox docs always returns true now. The boolHook is using .nodeValue.
comment:9 Changed 10 months ago by timmywil
@dmethvin: Actually, none of that is true.
As far as Sizzle.attr, notice that assertAttributes is true in Firefox so specified would never be used.
For the boolHook, typeof property is always "boolean" in Firefox so it would never get to it.
comment:10 Changed 10 months ago by dmethvin
Hmm, I see what you mean, it shouldn't be reaching those. So I wonder where are all these warnings coming from? I can't rebuild from master atm, so maybe this was for older code. How many warnings are you seeing in Firefox?
comment:11 Changed 10 months ago by timmywil
@dmethvin: As I said, the propHook for tabindex and the valHook for option elements. The latter is the use case for both of the tickets marked as duplicates of this ticket. There may be other cases that I missed, but I'm confident that if we adjust those two cases, we wouldn't see warnings anymore.
comment:12 Changed 9 months ago by timmywil
- Owner set to timmywil
- Status changed from open to assigned
comment:13 Changed 6 months ago by rhujer@…
Experiencing this issue too. Is there any progress in the last three months?
comment:14 Changed 5 months ago by jure.mav@…
Any progress on this issue?
comment:15 Changed 5 months ago by dmethvin
jure.mav, I don't know. Do you have some code to contribute?
comment:16 Changed 5 months ago by KP
I don't see any of these errors anymore in FF.
comment:17 Changed 5 months ago by dmethvin
These still occur in 1.9pre if "Show JavaScript Warnings" are enabled in Firefox 17.
@timmywil can you take a look?
comment:18 Changed 4 months ago by timmywil
Yea, I need to get on top of this. Will do tomorrow morning.
comment:19 Changed 4 months ago by dmethvin
- Milestone changed from 1.9 to 1.9.1
@timmywil another ping on this one, sorry. I'd like to get it wrapped up for 1.9.1 if possible.
comment:20 Changed 3 months ago by prashant sharma
@timmywil :- any updates on this one. Sorry to bug you for this bug :)
comment:22 Changed 2 months ago by timmywil
#13612 is a duplicate of this ticket.
comment:23 Changed 8 weeks ago by anonymous
Any news on that?
comment:24 Changed 8 weeks ago by anonymous
"I don't see any of these errors anymore in FF." - I don't know how that's possible. I see this error all the time in every version of Firefox I work with. Would be really good if someone have fixed that :)
comment:25 Changed 5 weeks ago by anonymous
There was no error in FF 19, but in FF 20 it showed up.
comment:26 Changed 5 weeks ago by anonymous
I'm seeing this error in Firefox 20
comment:27 Changed 5 weeks ago by Timmy Willison
- Status changed from assigned to closed
- Resolution set to fixed
Remove unnecessary usage of getAttributeNode(). Fixes #12072.
Changeset: 3ef7a9683b95fa334ab75458a531bb263782c748
Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.
