Bug Tracker

Opened 11 years ago

Closed 10 years ago

Last modified 9 years ago

#12072 closed bug (fixed)

Remove Firefox deprecated nodeValue, getAttributeNode, specified

Reported by: dmethvin Owned by: Timmy Willison
Priority: high Milestone: 1.10/2.0
Component: attributes Version: 1.7.2
Keywords: Cc:
Blocked by: Blocking:

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 (36)

comment:1 Changed 11 years ago by dmethvin

Component: unfiledattributes
Milestone: None1.9
Priority: undecidedhigh
Status: newopen

comment:2 Changed 11 years ago by dmethvin

#11397 is a duplicate of this ticket.

comment:3 Changed 11 years ago by dmethvin

#12162 is a duplicate of this ticket.

comment:4 Changed 11 years ago by dmethvin

Summary: Remove/avoid deprecated attribute featuresRemove Firefox deprecated nodeValue, getAttributeNode, specified

comment:5 Changed 11 years ago by Timmy Willison

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 11 years ago by Timmy Willison

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 11 years ago by Timmy Willison

Btw, Neither event.js nor native Sizzle uses the attributes property in FF anymore.

comment:8 Changed 11 years 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 11 years ago by Timmy Willison

@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 11 years 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 11 years ago by Timmy Willison

@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 11 years ago by Timmy Willison

Owner: set to Timmy Willison
Status: openassigned

comment:13 Changed 11 years ago by rhujer@…

Experiencing this issue too. Is there any progress in the last three months?

comment:14 Changed 10 years ago by jure.mav@…

Any progress on this issue?

comment:15 Changed 10 years ago by dmethvin

jure.mav, I don't know. Do you have some code to contribute?

comment:16 Changed 10 years ago by KP

I don't see any of these errors anymore in FF.

comment:17 Changed 10 years ago by dmethvin

These still occur in 1.9pre if "Show JavaScript Warnings" are enabled in Firefox 17.

http://i.imgur.com/ugHiT.png

@timmywil can you take a look?

comment:18 Changed 10 years ago by Timmy Willison

Yea, I need to get on top of this. Will do tomorrow morning.

comment:19 Changed 10 years ago by dmethvin

Milestone: 1.91.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 10 years ago by prashant sharma

@timmywil :- any updates on this one. Sorry to bug you for this bug :)

comment:21 Changed 10 years ago by dmethvin

Milestone: 1.9.1None

comment:22 Changed 10 years ago by Timmy Willison

#13612 is a duplicate of this ticket.

comment:23 Changed 10 years ago by anonymous

Any news on that?

comment:24 Changed 10 years 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 10 years ago by anonymous

There was no error in FF 19, but in FF 20 it showed up.

comment:26 Changed 10 years ago by anonymous

I'm seeing this error in Firefox 20

comment:27 Changed 10 years ago by Timmy Willison

Resolution: fixed
Status: assignedclosed

Remove unnecessary usage of getAttributeNode(). Fixes #12072.

Changeset: 3ef7a9683b95fa334ab75458a531bb263782c748

comment:28 Changed 10 years ago by Timmy Willison

Milestone: None1.10/2.0

comment:29 in reply to:  28 Changed 10 years ago by anonymous

Replying to timmywil: I still see this error in Firefox 21, jQuery 2.0.

comment:30 Changed 10 years ago by dmethvin

Try running without any add-ons. If you can repro without any add-ons using a page that simply includes jQuery, let us know.

comment:31 Changed 10 years ago by anonymous

Call to .specified near line...

jquery.js - Line: 2271

jQuery.extend({valHooks: {option: {get: function( elem ) {

attributes.value is undefined in Blackberry 4.7 but uses .value. See #6932 var val = elem.attributes.value;

return !val
val.specified ? elem.value : elem.text;

comment:32 Changed 10 years ago by dmethvin

@anonymous:31, do you have code that runs in Firefox that gets to this line? Can you share it?

comment:33 in reply to:  32 ; Changed 10 years ago by anonymous

Replying to dmethvin:

@anonymous:31, do you have code that runs in Firefox that gets to this line? Can you share it?

Apologies:

I went to put together a minimal test case to reproduce the error...and naturally double checked to make sure I was running the latest versions...and oops Feb.2013...So sorry...I see now the code has been modified in v1.10.1

comment:34 in reply to:  33 Changed 10 years ago by anonymous

Replying to anonymous:

Replying to dmethvin:

@anonymous:31, do you have code that runs in Firefox that gets to this line? Can you share it?

Apologies:

I went to put together a minimal test case to reproduce the error...and naturally double checked to make sure I was running the latest versions...and oops Feb.2013...So sorry...I see now the code has been modified in v1.10.1

v1.10.1 testing all good now.

comment:35 Changed 9 years ago by Erwan Legrand

I can reproduce this with jQuery-2.0.3 without any add-on: http://jsfiddle.net/NKBRe/1/

(The issue appears to be fixed in 1.10.1, though.)

comment:36 in reply to:  35 Changed 9 years ago by m_gol

Replying to Erwan Legrand:

I can reproduce this with jQuery-2.0.3 without any add-on: http://jsfiddle.net/NKBRe/1/

(The issue appears to be fixed in 1.10.1, though.)

Works fine on master: http://jsfiddle.net/NKBRe/3/ Please, always test on https://code.jquery.com/jquery-git2.js before reporting; on jsFiddle you can even choose "jQuery 2.x (edge)" from the list.

Note: See TracTickets for help on using tickets.