Bug Tracker

Ticket #7762 (closed enhancement: fixed)

Opened 4 years ago

Last modified 4 years ago

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

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.

Change History

comment:1 Changed 4 years ago by iliakan

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 ) {

comment:2 Changed 4 years ago by rwaldron

  • Owner set to iliakan
  • Status changed from new to pending

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

comment:3 Changed 4 years ago by iliakan

  • Status changed from pending to new

 Pull request is complete.

Last edited 4 years ago by jitter (previous) (diff)

comment:4 Changed 4 years ago by iliakan

Conditional comments can be replaced by

if ( && !window.XmlHttpRequest )

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

comment:5 Changed 4 years ago by iliakan

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

Last edited 4 years ago by iliakan (previous) (diff)

comment:6 Changed 4 years ago by snover

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to event
  • Milestone changed from 1.next to 1.5

comment:7 Changed 4 years ago by Colin Snover

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

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

Changeset: 429b078dc7b5cf7765b44c8d32e125ecd8f1e2c9

Note: See TracTickets for help on using tickets.