Skip to main content

Bug Tracker

Side navigation

#7762 closed enhancement (fixed)

Opened December 13, 2010 02:18PM UTC

Closed December 30, 2010 06:03AM UTC

[Events] Remove CPU overhead on unload

Reported by: iliakan Owned by: iliakan
Priority: low Milestone: 1.5
Component: event Version: 1.4.4
Keywords: Cc:
Blocked by: Blocking:
Description

There is a bug in IE6 which causes inter-page memory leaks. It is fixed in latest IE6 and IE7, in which the lesser, intra-page memory leak still exists.

The code in events.js fixes this bug by clobbering event handlers:

if ( window.attachEvent && !window.addEventListener) {
	jQuery(window).bind("unload", function() {
		alert(1)
		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) {}
			}
		}
	});
}

But actually it is required in IE6 only and causes extra CPU load in IE7, IE8.

Attachments (0)
Change History (7)

Changed December 13, 2010 02:20PM UTC by iliakan comment:1

The patch:

diff --git a/src/event.js b/src/event.js
index fd470e7..2b9696e 100644
--- a/src/event.js
+++ b/src/event.js
@@ -1181,7 +1181,7 @@ jQuery.each( ("blur focus focusin focusout load resize scroll unload click dblcl
 // 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 ) {
+if ( window.attachEvent && !window.addEventListener /*@cc_on @if (@_jscript_version > 5.6) && 0 /*@end @*/) {
 	jQuery(window).bind("unload", function() {
 		for ( var id in jQuery.cache ) {
 			if ( jQuery.cache[ id ].handle ) {

Changed December 13, 2010 04:22PM UTC by rwaldron comment:2

owner: → iliakan
status: newpending

Thanks for taking the time to contribute to the jQuery project! Please provide a reduced jsFiddle test case to help us assess your ticket!

Additionally, test against the latest jQuery release and the jQuery 0 GIT version to ensure the issue still exists. Be Excellent to eachother!

Additionally - patches should be submitted in the form of committed pull requests to the github repo, https://github.com/jquery/jquery

Changed December 14, 2010 08:28PM UTC by jitter comment:3

_comment0: Pull request is complete.1292358629135047
status: pendingnew

Pull request is complete.

Changed December 14, 2010 09:14PM UTC by iliakan comment:4

Conditional comments can be replaced by

if ( && !window.XmlHttpRequest )

The point is to prevent IE6 code from adding overhead on IE7 and IE8.

Changed December 18, 2010 11:52PM UTC by iliakan comment:5

_comment0: New pull: https://github.com/jquery/jquery/pull/1361292761354109009
_comment1: New pull, after discussion with rwaldron and joebourg: https://github.com/jquery/jquery/pull/1361292761515934522

New pull, after discussion with rwaldron and jaubourg: https://github.com/jquery/jquery/pull/136

Changed December 26, 2010 09:41PM UTC by snover comment:6

component: unfiledevent
milestone: 1.next1.5
priority: undecidedlow
status: newopen

Changed December 30, 2010 06:03AM UTC by Colin Snover comment:7

resolution: → fixed
status: openclosed

Merge branch 'fix7762' into jquery master. Fixes #7762.

Changeset: 429b078dc7b5cf7765b44c8d32e125ecd8f1e2c9