Bug Tracker

Ticket #8235 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

jQuery.data throws a script error in certain circumstances.

Reported by: julian.jelfs@… Owned by: julian.jelfs@…
Priority: low Milestone: 1.6.3
Component: data Version: 1.5
Keywords: Cc:
Blocking: Blocked by:

Description (last modified by jitter) (diff)

I'm not 100% of the circumstances under which this occurs and I cannot create a simple repro in jsFiddle.

However, there is a bit in jQuery.data as follows:

if ( (!id || (pvt && id && !cache[ id ][ internalKey ])) && getByName && data === undefined ) {
    return;
}

this is causing a script error when cache[id] is null (a condition that is checked for subsequently).

I am not sure if it is legitimate that we should get to this point and cache[id] is null, but it is certainly happening and only started happening when upgraded to jQuery 1.5.

Seems that the code could be more defensive to prevent the script error (which is what I have done for now in my local copy).

Change History

comment:1 Changed 4 years ago by jitter

  • Owner set to julian.jelfs@…
  • Status changed from new to pending

Thanks for taking the time to contribute to the jQuery project by writing a bug report.

Do you use jQuery UI on the same page? If yes could you try including  http://code.jquery.com/jquery-git.js (instead of 1.5) on that page and then check if the error still occurs and then report back if that fixed it for you?

Because there is a known regression in 1.5 which affects jQuery UI and currently it's also not advised/supported to use jQuery UI with jQuery 1.5 .

Probably a duplicate of #8123

comment:2 Changed 4 years ago by julian.jelfs@…

  • Status changed from pending to new

I tried with the suggested version of jquery and I still get the same error.

I am using jquery UI 1.8.6. I will also try upgrading to the 1.8.9.

In the meantime I have just added:

if (id && !cache[ id ] ) {

cache[ id ] = {};

}

before the statement that causes the error and everything seems fine but I don't know if this will have any other unwanted side effects.

Thanks,

Julian.

comment:3 Changed 4 years ago by jitter

  • Status changed from new to pending
  • Component changed from unfiled to data
  • Description modified (diff)

Without any further information / test case or at least a link to a page where the issue can reproduced we can't take any further steps on this.

So please report back if you can provide any of this so the we can further investigate this issue.

comment:4 Changed 4 years ago by anonymous

I am seeing this exact issue also. It seems to only error on Internet Explorer. I am using IE7. Works successfully on Firefox and Chrome. That might provide a clue to recreating the issue.

comment:5 Changed 4 years ago by gmc

I've got a cut-down version of a page in which I was experiencing this problem - issue only appears in IE, not in FF or Chrome. Also doesn't appear within jsFiddle in IE for some reason - hence link to html fiel below.

There is a link to the recreated issue below. The qunit test passes when the page is changed to point at a patched version of jquery 1.5.1 in which the line:

if ( (!id || (pvt && id && !cache[ id ][ internalKey ])) && getByName && data === undefined ) {

is changed to:

if ( (!id || (pvt && id && cache[ id ] && !cache[ id ][ internalKey ])) && getByName && data === undefined ) {

Page recreating issue:  https://docs.google.com/leaf?id=0BxFi8HJljH7LN2YzNzg1ZGQtMmVhMC00ZDg4LWE5NDEtMDI0MDQ2MDlkNjkz&hl=en&authkey=CN-Ik8kF

comment:6 Changed 4 years ago by trac-o-bot

  • Status changed from pending to closed
  • Resolution set to invalid

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

comment:7 Changed 3 years ago by niebieskiii@…

I have the same issue that exists only in IE7 and only with jquery version grater than 1.4.4

Test case:  http://jsfiddle.net/ZP2T6/

  • run it under ie7
  • add 2 items by clicking 'add new'
  • remove 2 items by clicking 'remove'

you get error on page in jquery line mentioned earlier in this topic

comment:8 Changed 3 years ago by dalugadm

I too am seeing this issue. It appears to happen in IE7 and IE8 (I have not tried IE9). FF does not appear to have any problems. I am using 1.6.1.

The example provided by niebieskiii seems to reproduce the problem. Is there any plans to investigate this issue further? I have modified my local copy to make it work but would prefer a JQuery accepted solution so that it has been properly regression tested.

comment:9 Changed 3 years ago by rwaldron

This:  http://jsfiddle.net/ZP2T6/ is not a reduced test case of which we can base an attempt to fix the problem on...

Please reduce this further without jQuery UI present

comment:10 Changed 3 years ago by dalugadm

In my limited testing, once I remove JQuery UI, the problem appears to disappear. So, at first glance, it appears to be some type of interaction between JQuery and JQuery UI.

Since I am new here, what would be the next step? I appreciate your time in helping with this!

comment:11 Changed 3 years ago by rwaldron

That's very helpful, thank you. You'll want to refile this ticket here:  http://bugs.jqueryui.com/

comment:12 Changed 3 years ago by rwaldron

  • Priority changed from undecided to low

comment:13 Changed 3 years ago by dalugadm

Thank you rwaldron!

For anyone interested, I have refiled the ticket here:  http://bugs.jqueryui.com/ticket/7510

comment:14 Changed 3 years ago by scott.gonzalez

  • Status changed from closed to reopened
  • Resolution invalid deleted

 https://github.com/jquery/jquery/blob/b22c9046529852c7ce56/src/data.js#L54

cache[ id ][ internalKey ] is checked without ensuring that cache[ id ] exists.

I'm not really sure how we end up in that situation, but this should be fixed in core.

comment:15 Changed 3 years ago by timmywil

  • Status changed from reopened to open

comment:16 Changed 3 years ago by timmywil

  • Milestone changed from 1.next to 1.6.3

comment:17 Changed 3 years ago by rwaldron

#10061 is a duplicate of this ticket.

comment:18 Changed 3 years ago by rwaldron

#10080 is a duplicate of this ticket.

comment:19 Changed 3 years ago by rwaldron

#10080 is a duplicate in that it is the same behaviour, different location in the code

comment:20 Changed 3 years ago by Rick Waldron

  • Status changed from open to closed
  • Resolution set to fixed

Landing pull request 459. Do not allow assumed cache[id] in jQuery.data. Fixes #8235.

More Details:

Note: See TracTickets for help on using tickets.