Side navigation
#10080 closed bug (fixed)
Opened August 17, 2011 08:47PM UTC
Closed August 25, 2011 07:18PM UTC
Last modified September 01, 2011 05:02PM UTC
unload from frame's window breaks in IE8
| Reported by: | brian.moschel@gmail.com | Owned by: | rwaldron |
|---|---|---|---|
| Priority: | low | Milestone: | 1.6.3 |
| Component: | data | Version: | git |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
If you have an iframe in the same domain and try to bind to unload:
$(someotherwindow).unload(fn)
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
Attachments (0)
Change History (15)
Changed August 17, 2011 08:49PM UTC by comment:1
Changed August 17, 2011 09:02PM UTC by comment:2
| component: | unfiled → data |
|---|---|
| milestone: | None → 1.6.3 |
| priority: | undecided → low |
| resolution: | → duplicate |
| status: | new → closed |
Changed August 17, 2011 09:12PM UTC by comment:4
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.
Changed August 17, 2011 09:13PM UTC by comment:5
That code sample should show:
if ( jQuery.support.deleteExpando || cache != window )
Changed August 17, 2011 09:44PM UTC by comment:6
| resolution: | duplicate |
|---|---|
| status: | closed → reopened |
Or you could simply ensure that cache[ id ] actually exists...
Which is the same issue.
Changed August 17, 2011 09:44PM UTC by comment:7
Aso, see my comment here: http://bugs.jquery.com/ticket/8235#comment:19
Changed August 18, 2011 02:33AM UTC by comment:8
Is this a result of not having a test case for #8235? We fixed that problem without knowing the cause.
Changed August 18, 2011 06:38AM UTC by comment:9
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.
Changed August 18, 2011 01:02PM UTC by comment:10
@dmethvin, no they are actually unrelated.
Changed August 18, 2011 02:21PM UTC by comment:11
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.
Changed August 18, 2011 02:30PM UTC by comment:12
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.
Changed August 25, 2011 02:43PM UTC by comment:13
| owner: | → rwaldron |
|---|---|
| status: | reopened → assigned |
Fixed by https://github.com/jquery/jquery/pull/468 .
Changed August 25, 2011 07:18PM UTC by comment:14
Changed September 01, 2011 05:02PM UTC by comment:15
#8870 is a duplicate of this ticket.
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.