diff --git a/src/jqLite.js b/src/jqLite.js index 3eee01f507d1..1fa0f1d64765 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -161,6 +161,12 @@ function jqLiteIsTextNode(html) { return !HTML_REGEXP.test(html); } +function jqLiteAcceptsData(node) { + // The window object can accept data but has no nodeType + // Otherwise we are only interested in elements (1) and documents (9) + return !node.nodeType || node.nodeType === 1 || node.nodeType === 9; +} + function jqLiteBuildFragment(html, context) { var elem, tmp, tag, wrap, fragment = context.createDocumentFragment(), @@ -308,30 +314,29 @@ function jqLiteExpandoStore(element, key, value) { } function jqLiteData(element, key, value) { - var data = jqLiteExpandoStore(element, 'data'), - isSetter = isDefined(value), - keyDefined = !isSetter && isDefined(key), - isSimpleGetter = keyDefined && !isObject(key); - - if (!data && !isSimpleGetter) { - jqLiteExpandoStore(element, 'data', data = {}); - } + if (jqLiteAcceptsData(element)) { + var data = jqLiteExpandoStore(element, 'data'), + isSetter = isDefined(value), + keyDefined = !isSetter && isDefined(key), + isSimpleGetter = keyDefined && !isObject(key); + + if (!data && !isSimpleGetter) { + jqLiteExpandoStore(element, 'data', data = {}); + } - if (isSetter) { - // set data only on Elements and Documents - if (element.nodeType === 1 || element.nodeType === 9) { + if (isSetter) { data[key] = value; - } - } else { - if (keyDefined) { - if (isSimpleGetter) { - // don't create data in this case. - return data && data[key]; + } else { + if (keyDefined) { + if (isSimpleGetter) { + // don't create data in this case. + return data && data[key]; + } else { + extend(data, key); + } } else { - extend(data, key); + return data; } - } else { - return data; } } } @@ -750,6 +755,11 @@ forEach({ on: function onFn(element, type, fn, unsupported){ if (isDefined(unsupported)) throw jqLiteMinErr('onargs', 'jqLite#on() does not support the `selector` or `eventData` parameters'); + // Do not add event handlers to non-elements because they will not be cleaned up. + if (!jqLiteAcceptsData(element)) { + return; + } + var events = jqLiteExpandoStore(element, 'events'), handle = jqLiteExpandoStore(element, 'handle'); diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index ffe02570f59d..cf880aef0382 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -386,6 +386,23 @@ describe('jqLite', function() { selected.removeData('prop2'); }); + + it('should not add to the cache if the node is a comment or text node', function() { + var calcCacheSize = function() { + var count = 0; + for (var k in jqLite.cache) { ++count; } + return count; + }; + + var nodes = jqLite(' and some text'); + expect(calcCacheSize()).toEqual(0); + nodes.data('someKey'); + expect(calcCacheSize()).toEqual(0); + nodes.data('someKey', 'someValue'); + expect(calcCacheSize()).toEqual(0); + }); + + it('should emit $destroy event if element removed via remove()', function() { var log = ''; var element = jqLite(a); @@ -993,6 +1010,16 @@ describe('jqLite', function() { expect(log).toEqual('click on: A;click on: B;'); }); + it('should not bind to comment or text nodes', function() { + var nodes = jqLite('Some text'); + var someEventHandler = jasmine.createSpy('someEventHandler'); + + nodes.on('someEvent', someEventHandler); + nodes.triggerHandler('someEvent'); + + expect(someEventHandler).not.toHaveBeenCalled(); + }); + it('should bind to all events separated by space', function() { var elm = jqLite(a), callback = jasmine.createSpy('callback'); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 5de1728a262b..f72c1fd8001e 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3928,6 +3928,66 @@ describe('$compile', function() { }); + + it('should not leak if two "element" transclusions are on the same element', function() { + var calcCacheSize = function() { + var size = 0; + forEach(jqLite.cache, function(item, key) { size++; }); + return size; + }; + + inject(function($compile, $rootScope) { + expect(calcCacheSize()).toEqual(0); + + element = $compile('
{{x}}
')($rootScope); + expect(calcCacheSize()).toEqual(1); + + $rootScope.$apply('xs = [0,1]'); + expect(calcCacheSize()).toEqual(2); + + $rootScope.$apply('xs = [0]'); + expect(calcCacheSize()).toEqual(1); + + $rootScope.$apply('xs = []'); + expect(calcCacheSize()).toEqual(1); + + element.remove(); + expect(calcCacheSize()).toEqual(0); + }); + }); + + + it('should not leak if two "element" transclusions are on the same element', function() { + var calcCacheSize = function() { + var size = 0; + forEach(jqLite.cache, function(item, key) { size++; }); + return size; + }; + inject(function($compile, $rootScope) { + expect(calcCacheSize()).toEqual(0); + element = $compile('
{{x}}
')($rootScope); + + $rootScope.$apply('xs = [0,1]'); + // At this point we have a bunch of comment placeholders but no real transcluded elements + // So the cache only contains the root element's data + expect(calcCacheSize()).toEqual(1); + + $rootScope.$apply('val = true'); + // Now we have two concrete transcluded elements plus some comments so two more cache items + expect(calcCacheSize()).toEqual(3); + + $rootScope.$apply('val = false'); + // Once again we only have comments so no transcluded elements and the cache is back to just + // the root element + expect(calcCacheSize()).toEqual(1); + + element.remove(); + // Now we've even removed the root element along with its cache + expect(calcCacheSize()).toEqual(0); + }); + }); + + it('should remove transclusion scope, when the DOM is destroyed', function() { module(function() { directive('box', valueFn({ @@ -4409,6 +4469,35 @@ describe('$compile', function() { $rootScope.$digest(); expect(element.text()).toEqual('transcluded content'); })); + + + it('should not leak memory with nested transclusion', function() { + var calcCacheSize = function() { + var count = 0; + for (var k in jqLite.cache) { ++count; } + return count; + }; + + inject(function($compile, $rootScope) { + var size; + + expect(calcCacheSize()).toEqual(0); + + element = jqLite('
'); + $compile(element)($rootScope.$new()); + + $rootScope.nums = [0,1,2]; + $rootScope.$apply(); + size = calcCacheSize(); + + $rootScope.nums = [3,4,5]; + $rootScope.$apply(); + expect(calcCacheSize()).toEqual(size); + + element.remove(); + expect(calcCacheSize()).toEqual(0); + }); + }); });