Bug Tracker

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10080 closed bug (fixed)

unload from frame's window breaks in IE8

Reported by: brian.moschel@… Owned by: Rick Waldron
Priority: low Milestone: 1.6.3
Component: data Version: git
Keywords: Cc:
Blocked by: Blocking:


If you have an iframe in the same domain and try to bind to unload:


This works in all browsers except IE8.


The problem is when you bind to unload, it uses the "one" method, so after unload, it unbinds itself and calls jquery.removeData, which has this:

// Browsers that fail expando deletion also refuse to delete expandos on
// the window, but it will allow it on all other JS objects; other browsers
// don't care
if ( jQuery.support.deleteExpando || cache != window ) {
  delete cache[ id ];
} else {
  cache[ id ] = null;

That first if is true for IE8 and below I believe. Cache isn't the current window in this case, its another frame's window object, so delete cache[id] throws an error, and the unload event handler isn't reached

The fix is to check if cache is any window, not just the current window

Change History (15)

comment:1 Changed 6 years ago by anonymous

I should say the error thrown is "Object doesn't support this action" and at least in 1.6.2 its on delete cache[id].

The error prevents the event handler from being called at all.

comment:2 Changed 6 years ago by Rick Waldron

Component: unfileddata
Milestone: None1.6.3
Priority: undecidedlow
Resolution: duplicate
Status: newclosed

comment:3 Changed 6 years ago by Rick Waldron

Duplicate of #8235.

comment:4 Changed 6 years ago by brian.moschel@…

I don't think those 2 bugs are duplicates. In 8235, they said:

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

In the error I'm describing, the problem is

if ( jQuery.support.deleteExpando
cache != window )

is true if cache is another frame's window, but delete cache[id] breaks. That if statement should check if cache is ANY window, not just the current one. Those are not the same cases.

comment:5 Changed 6 years ago by brian.moschel@…

That code sample should show:

if ( jQuery.support.deleteExpando || cache != window )

comment:6 Changed 6 years ago by Rick Waldron

Resolution: duplicate
Status: closedreopened

Or you could simply ensure that cache[ id ] actually exists...

Which is the same issue.


comment:7 Changed 6 years ago by Rick Waldron

comment:8 Changed 6 years ago by dmethvin

Is this a result of not having a test case for #8235? We fixed that problem without knowing the cause.

comment:9 Changed 6 years ago by brian.moschel@…

That pull request doesn't actually address the issue I'm talking about. The core issue is that cache can be any window object.

I made a pull request with a test that currently breaks in IE8 and a fix.


comment:10 Changed 6 years ago by Rick Waldron

@dmethvin, no they are actually unrelated.

comment:11 Changed 6 years ago by Rick Waldron

Using jQuery.isWindow will be significantly slower and all calls to removeData will be affected, see: http://jsperf.com/iswindow-vs-inline-test

I've updated my branch to test for a window inference and have included your test with credit to you.

comment:12 Changed 6 years ago by dmethvin

Yes .isWindow() is slower but look at the numbers. Even the "slow" path can run almost 2 *million* times a second. It's not likely the .removeData() path is critical enough that the difference matters.

comment:13 Changed 6 years ago by dmethvin

Owner: set to Rick Waldron
Status: reopenedassigned

comment:14 Changed 6 years ago by Dave Methvin

Resolution: fixed
Status: assignedclosed

Merge pull request #468 from rwldrn/10080

Fixes #10080. Test cache for window inference.

Changeset: f4811bfb04206b9d9c120001603eadb1b529c271

comment:15 Changed 6 years ago by dmethvin

#8870 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.