IE memory leak in jquery support function

[agnes@…]

The following code in the jquery support detection function will cause a memory leak in IE.

if ( !div.addEventListener && div.attachEvent && div.fireEvent ) {
		div.attachEvent("onclick", function click() {
			// Cloning a node shouldn't copy over any
			// bound event handlers (IE does this)
			jQuery.support.noCloneEvent = false;
			div.detachEvent("onclick", click);
		});
		div.cloneNode(true).fireEvent("onclick");
	}

This is because the detachEvent is only ever called on the div and not never on the cloned div.

It can be fixed simply with this code:

    if ( !div.addEventListener && div.attachEvent && div.fireEvent ) {
        var click = function() {
			// Cloning a node shouldn't copy over any
			// bound event handlers (IE does this)
			jQuery.support.noCloneEvent = false;
			this.detachEvent("onclick", click);
		};
		div.attachEvent("onclick", click);
		div.cloneNode(true).fireEvent("onclick");
        div.detachEvent("onclick", click);
	}

[timmywil]

That makes sense to me. Can we confirm the leak in IE?

[dmethvin]

yes, memory leaks can be elusive so a test case is appreciated.

[trac-o-bot]

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

[shghs]

The leak is present in IE6 and IE7. A test case can be produced by including jQuery in a blank HTML page with no other JavaScript. Reloading the page causes IE's memory usage to increase constantly. With the fix that Agnes suggests, the same test case doesn't result in memory leakage.

I've issued a pull request: ​https://github.com/jquery/jquery/pull/387

[Rick Waldron]

This might be more effective and has less change to the existing code:

​https://github.com/rwldrn/jquery/commit/34e6c0637bb8ac9420761232078ccfcc15ca74ad

[timmywil]

I'm thinking both should be done, but I'll do some testing.

[timmywil]

detachEvent is unnecessary since we're nulling div. Fixes #8873.

Changeset: b8fc9d14a134f91f17bf96194da86d38231da005