Skip to content

Commit

Permalink
Fix #8545. Plug event handling memory leak in oldIE.
Browse files Browse the repository at this point in the history
  • Loading branch information
markelog authored and dmethvin committed Apr 5, 2012
1 parent 41056ab commit 203a168
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 19 deletions.
6 changes: 5 additions & 1 deletion src/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ var rbrace = /^(?:\{.*\}|\[.*\])$/,
jQuery.extend({
cache: {},

deletedIds: [],

// Please use with caution
uuid: 0,

Expand Down Expand Up @@ -59,7 +61,7 @@ jQuery.extend({
// Only DOM nodes need a new unique ID for each element since their data
// ends up in the global cache
if ( isNode ) {
elem[ internalKey ] = id = ++jQuery.uuid;
elem[ internalKey ] = id = jQuery.deletedIds.pop() || ++jQuery.uuid;
} else {
id = internalKey;
}
Expand Down Expand Up @@ -212,6 +214,8 @@ jQuery.extend({
// We destroyed the cache and need to eliminate the expando on the node to avoid
// false lookups in the cache for entries that no longer exist
if ( isNode ) {
jQuery.deletedIds.push( id );

// IE does not allow us to delete expando properties from nodes,
// nor does it have a removeAttribute function on Document nodes;
// we must handle all of these cases
Expand Down
20 changes: 13 additions & 7 deletions src/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ jQuery.event = {

var elemData = jQuery.hasData( elem ) && jQuery._data( elem ),
t, tns, type, origType, namespaces, origCount,
j, events, special, handle, eventType, handleObj;
j, events, special, eventType, handleObj;

if ( !elemData || !(events = elemData.events) ) {
return;
Expand Down Expand Up @@ -213,14 +213,11 @@ jQuery.event = {

// Remove the expando if it's no longer used
if ( jQuery.isEmptyObject( events ) ) {
handle = elemData.handle;
if ( handle ) {
handle.elem = null;
}
delete elemData.handle;

// removeData also checks for emptiness and clears the expando if empty
// so use it instead of delete
jQuery.removeData( elem, [ "events", "handle" ], true );
jQuery.removeData( elem, "events", true );
}
},

Expand Down Expand Up @@ -636,8 +633,17 @@ jQuery.removeEvent = document.removeEventListener ?
}
} :
function( elem, type, handle ) {
var name = "on" + type;

if ( elem.detachEvent ) {
elem.detachEvent( "on" + type, handle );

// #8545, #7054, preventing memory leaks for custom events in IE6-8 –
// detachEvent needed property on element, by name of that event, to properly expose it to GC
if ( typeof elem[ name ] === "undefined" ) {
elem[ name ] = null;
}

elem.detachEvent( name, handle );
}
};

Expand Down
21 changes: 10 additions & 11 deletions src/manipulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -811,21 +811,20 @@ jQuery.extend({
jQuery.removeEvent( elem, type, data.handle );
}
}

// Null the DOM reference to avoid IE6/7/8 leak (#7054)
if ( data.handle ) {
data.handle.elem = null;
}
}

if ( deleteExpando ) {
delete elem[ jQuery.expando ];
// Remove cache only if jQuery.event.remove was not removed it before
if ( cache[ id ] ) {
if ( deleteExpando ) {
delete elem[ jQuery.expando ];

} else if ( elem.removeAttribute ) {
elem.removeAttribute( jQuery.expando );
}
} else if ( elem.removeAttribute ) {
elem.removeAttribute( jQuery.expando );
}

delete cache[ id ];
jQuery.deletedIds.push( id );
delete cache[ id ];
}
}
}
}
Expand Down

0 comments on commit 203a168

Please sign in to comment.