Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 462dbb2

Browse files
fix(jqLite): don't attach event handlers to comments or text nodes
We were attaching handlers to comment nodes when setting up bound transclusion functions. But we don't clean up comments and text nodes when deallocating so there was a memory leak. Closes #7913 Closes #7942
1 parent 7f63e81 commit 462dbb2

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

src/jqLite.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,12 @@ function jqLiteIsTextNode(html) {
161161
return !HTML_REGEXP.test(html);
162162
}
163163

164+
function jqLiteAcceptsData(node) {
165+
// The window object can accept data but has no nodeType
166+
// Otherwise we are only interested in elements (1) and documents (9)
167+
return !node.nodeType || node.nodeType === 1 || node.nodeType === 9;
168+
}
169+
164170
function jqLiteBuildFragment(html, context) {
165171
var elem, tmp, tag, wrap,
166172
fragment = context.createDocumentFragment(),
@@ -319,7 +325,7 @@ function jqLiteData(element, key, value) {
319325

320326
if (isSetter) {
321327
// set data only on Elements and Documents
322-
if (element.nodeType === 1 || element.nodeType === 9) {
328+
if (jqLiteAcceptsData(element)) {
323329
data[key] = value;
324330
}
325331
} else {
@@ -754,6 +760,11 @@ forEach({
754760
on: function onFn(element, type, fn, unsupported){
755761
if (isDefined(unsupported)) throw jqLiteMinErr('onargs', 'jqLite#on() does not support the `selector` or `eventData` parameters');
756762

763+
// Do not add event handlers to non-elements because they will not be cleaned up.
764+
if (!jqLiteAcceptsData(element)) {
765+
return;
766+
}
767+
757768
var events = jqLiteExpandoStore(element, 'events'),
758769
handle = jqLiteExpandoStore(element, 'handle');
759770

test/jqLiteSpec.js

+10
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,16 @@ describe('jqLite', function() {
993993
expect(log).toEqual('click on: A;click on: B;');
994994
});
995995

996+
it('should not bind to comment or text nodes', function() {
997+
var nodes = jqLite('<!-- some comment -->Some text');
998+
var someEventHandler = jasmine.createSpy('someEventHandler');
999+
1000+
nodes.on('someEvent', someEventHandler);
1001+
nodes.triggerHandler('someEvent');
1002+
1003+
expect(someEventHandler).not.toHaveBeenCalled();
1004+
});
1005+
9961006
it('should bind to all events separated by space', function() {
9971007
var elm = jqLite(a),
9981008
callback = jasmine.createSpy('callback');

0 commit comments

Comments
 (0)