Skip to content

Commit a3d0964

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 angular#7913
1 parent 1a87904 commit a3d0964

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 {
@@ -750,6 +756,11 @@ forEach({
750756
on: function onFn(element, type, fn, unsupported){
751757
if (isDefined(unsupported)) throw jqLiteMinErr('onargs', 'jqLite#on() does not support the `selector` or `eventData` parameters');
752758

759+
// Do not add event handlers to non-elements because they will not be cleaned up.
760+
if (!jqLiteAcceptsData(element)) {
761+
return;
762+
}
763+
753764
var events = jqLiteExpandoStore(element, 'events'),
754765
handle = jqLiteExpandoStore(element, 'handle');
755766

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)