Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#10266 closed bug (wontfix)

Undefined values passed to $(element).data(key,value) break chaining

Reported by: jsoverson@… Owned by:
Priority: low Milestone: None
Component: data Version: 1.6.4rc1
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by Rick Waldron)

Example here:

The problem occurs when jquery tests for definedness of the passed value. If it is undefined the logic assumes you want the value of the key, which is likely not the assumption a user makes when they pass in two arguments. The problem experienced here is largest during chaining where the returned value breaks the rest of the chain.

This issue is also what likely triggered ticket but that ticket was closed prematurely due to the inability to reproduce.

Change History (7)

comment:1 Changed 7 years ago by james_padolsey

jQuery.fn.data appears to check for the presence of the value argument by the expression value !== undefined. Another way would be to check the arguments.length, but if that was done then $(el).data('a', undefined) would appear to set a to undefined, but it doesn't (jQuery.data also uses the arg === undefined test). So, if a was already set to 123, it'll remain that way. IMO, this'd be misleading. The question is: should passing an argument, even if it's undefined, still be construed as intentionally passing that argument, or alternatively: willfully accessing the getter functionality of jQuery.fn.data??

What should jQuery(el).data('foo', undefined) be expected to do?

Anyway, like I said, a fix would be easy enough -- checking for the appropriate argument-number in arguments.

comment:2 Changed 7 years ago by anonymous

I think setting a value to undefined is a valid use case, but that might be a different discussion.

I think, regardless, by sending an argument in place of the "value" field, retrieving the previously set value is the least expected result.

I threw in the fix and test case here since I was already working in the code: https://github.com/jquery/jquery/pull/501

comment:3 Changed 7 years ago by timmywil

Component: unfileddata
Priority: undecidedlow

All jQuery getters/setters check for undefined. Retrieving an item specifically set to undefined and trying to retrieve one that doesn't exist will produce exactly the same result, so there shouldn't be a reason to set data to undefined. I say use removeData.

comment:4 Changed 7 years ago by Rick Waldron

Description: modified (diff)

-1 This request has been made before for other methods and we've argued the color of the explicit undefined parameter bikeshed plenty of times.

comment:5 Changed 7 years ago by Rick Waldron

Resolution: wontfix
Status: newclosed

comment:6 Changed 7 years ago by Billie Cleek

While I understand the decision to not fix this issue, the documentation for .removeData() explicitly says setting value to undefined is one of the preferred ways of ensuring that the data value is not reset to the value of the data- attribute.

comment:7 Changed 7 years ago by ajpiano

This actually seems like it's now fixed as part of #5571 getting fixed. Can you check in the latest edge version please?

Note: See TracTickets for help on using tickets.