Skip to content

Commit f768c41

Browse files
petebacondarwinCameron Knight
authored and
Cameron Knight
committed
fix(jqLite): never add to the cache for non-element/document nodes
Calling `jqLite.data()` on a disallowed node type caused an empty object to be added to the cache. This could lead to memory leaks since we no longer clean up such node types when they are removed from the DOM. Closes angular#7966
1 parent 95fbed5 commit f768c41

File tree

2 files changed

+35
-19
lines changed

2 files changed

+35
-19
lines changed

src/jqLite.js

+18-19
Original file line numberDiff line numberDiff line change
@@ -314,30 +314,29 @@ function jqLiteExpandoStore(element, key, value) {
314314
}
315315

316316
function jqLiteData(element, key, value) {
317-
var data = jqLiteExpandoStore(element, 'data'),
318-
isSetter = isDefined(value),
319-
keyDefined = !isSetter && isDefined(key),
320-
isSimpleGetter = keyDefined && !isObject(key);
317+
if (jqLiteAcceptsData(element)) {
318+
var data = jqLiteExpandoStore(element, 'data'),
319+
isSetter = isDefined(value),
320+
keyDefined = !isSetter && isDefined(key),
321+
isSimpleGetter = keyDefined && !isObject(key);
321322

322-
if (!data && !isSimpleGetter) {
323-
jqLiteExpandoStore(element, 'data', data = {});
324-
}
323+
if (!data && !isSimpleGetter) {
324+
jqLiteExpandoStore(element, 'data', data = {});
325+
}
325326

326-
if (isSetter) {
327-
// set data only on Elements and Documents
328-
if (jqLiteAcceptsData(element)) {
327+
if (isSetter) {
329328
data[key] = value;
330-
}
331-
} else {
332-
if (keyDefined) {
333-
if (isSimpleGetter) {
334-
// don't create data in this case.
335-
return data && data[key];
329+
} else {
330+
if (keyDefined) {
331+
if (isSimpleGetter) {
332+
// don't create data in this case.
333+
return data && data[key];
334+
} else {
335+
extend(data, key);
336+
}
336337
} else {
337-
extend(data, key);
338+
return data;
338339
}
339-
} else {
340-
return data;
341340
}
342341
}
343342
}

test/jqLiteSpec.js

+17
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,23 @@ describe('jqLite', function() {
386386
selected.removeData('prop2');
387387
});
388388

389+
390+
it('should not add to the cache if the node is a comment or text node', function() {
391+
var calcCacheSize = function() {
392+
var count = 0;
393+
for (var k in jqLite.cache) { ++count; }
394+
return count;
395+
};
396+
397+
var nodes = jqLite('<!-- some comment --> and some text');
398+
expect(calcCacheSize()).toEqual(0);
399+
nodes.data('someKey');
400+
expect(calcCacheSize()).toEqual(0);
401+
nodes.data('someKey', 'someValue');
402+
expect(calcCacheSize()).toEqual(0);
403+
});
404+
405+
389406
it('should emit $destroy event if element removed via remove()', function() {
390407
var log = '';
391408
var element = jqLite(a);

0 commit comments

Comments
 (0)