#6452 closed bug (fixed)
Window unload events could be removed before handlers are executed
Reported by: | pinkhominid | Owned by: | |
---|---|---|---|
Priority: | undecided | Milestone: | |
Component: | event | Version: | 1.4.2 |
Keywords: | ie leak window unload | Cc: | |
Blocked by: | Blocking: |
Description
The "Prevent memory leaks in IE" code block attaches a function to clean up events on window unload. Since browsers don't guarantee the order of event handler execution, this code can sometimes run before other window unload handlers and remove them before they have a chance to run.
There is a comment that states " Window isn't included so as not to unbind existing unload events". This is true for jQ 1.3.2, but is missing for 1.4.2.
The 1.3.2 code to do this excluded the first item in jQuery.event, the handle of which was always the window object. The handle is not the window object in 1.4.2.
So it makes sense that this disappeared, however, the comment stayed and the code doesn't currently reflect that.
A proposed fix starts at line 2608 of jQuery 1.4.2.
Change this:
// Prevent memory leaks in IE // Window isn't included so as not to unbind existing unload events // More info: // - http://isaacschlueter.com/2006/10/msie-memory-leaks/ if ( window.attachEvent && !window.addEventListener ) { window.attachEvent("onunload", function() { for ( var id in jQuery.cache ) { if ( jQuery.cache[ id ].handle ) { // Try/Catch is to handle iframes being unloaded, see #4280 try { jQuery.event.remove( jQuery.cache[ id ].handle.elem ); } catch(e) {} } } }); }
To this:
// Prevent memory leaks in IE // Window isn't included so as not to unbind existing unload events // More info: // - http://isaacschlueter.com/2006/10/msie-memory-leaks/ if ( window.attachEvent && !window.addEventListener ) { window.attachEvent("onunload", function() { for ( var id in jQuery.cache ) { var item = jQuery.cache[ id ]; if ( item.handle ) { if ( item.handle.elem === window ) { for ( var type in item.events ) { if ( type !== "unload" ) { // Try/Catch is to handle iframes being unloaded, see #4280 try { jQuery.event.remove( item.handle.elem, type ); } catch(e) {} } } } else { // Try/Catch is to handle iframes being unloaded, see #4280 try { jQuery.event.remove( item.handle.elem ); } catch(e) {} } } } }); }
Demo:
Change History (5)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
Thanks a lot for this bug report! You saved my life. I have faced this bug a month ago, but nobody could help me. See my post here
http://groups.google.com/group/jquery-ui-layout/browse_thread/thread/534aa09a10a621ba
I'm waiting now for jQuery 1.4.3.
comment:4 Changed 12 years ago by
Priority: | → undecided |
---|---|
Status: | new → open |
comment:5 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
This code was only required for IE6 SP1, which is no longer supported by Microsoft, man, or beast. IE6 with XP SP2 or above does not leak for this case, which was a security/privacy issue as well. The problem was fixed in jQuery 1.4.3.
well done ...
I come across this code