Bug Tracker

Ticket #6452 (closed bug: fixed)

Opened 4 years ago

Last modified 2 years ago

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:
Blocking: Blocked by:

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:

 http://jsbin.com/uquwu3

Change History

comment:1 Changed 4 years ago by caii

well done ...

I come across this code

<script type="text/javascript">
window.attachEvent("onunload", function() {  alert("onunload fired") });
</script>
<script type="text/javascript" src="jquery1.4.2.js"></script>
<script type="text/javascript">
$(window).unload(function(){alert('页面已经关闭')})
</script>

comment:2 Changed 4 years ago by ova2

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:3 Changed 4 years ago by snover

  • Milestone 1.4.3 deleted

Resetting milestone to future.

comment:4 Changed 4 years ago by dmethvin

  • Priority set to undecided
  • Status changed from new to open

comment:5 Changed 3 years ago by dmethvin

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

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.

Note: See TracTickets for help on using tickets.