diff options
| -rw-r--r-- | src/ng/compile.js | 9 | ||||
| -rw-r--r-- | src/ng/directive/ngView.js | 2 | ||||
| -rw-r--r-- | test/ng/compileSpec.js | 42 | ||||
| -rw-r--r-- | test/ng/directive/ngViewSpec.js | 26 |
4 files changed, 75 insertions, 4 deletions
diff --git a/src/ng/compile.js b/src/ng/compile.js index b7eb759b..788231ab 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -330,7 +330,14 @@ function $CompileProvider($provide) { var $linkNode = cloneConnectFn ? JQLitePrototype.clone.call($compileNodes) // IMPORTANT!!! : $compileNodes; - $linkNode.data('$scope', scope); + + // Attach scope only to non-text nodes. + for(var i = 0, ii = $linkNode.length; i<ii; i++) { + var node = $linkNode[i]; + if (node.nodeType == 1 /* element */ || node.nodeType == 9 /* document */) { + $linkNode.eq(i).data('$scope', scope); + } + } safeAddClass($linkNode, 'ng-scope'); if (cloneConnectFn) cloneConnectFn($linkNode, scope); if (compositeLinkFn) compositeLinkFn(scope, $linkNode, $linkNode); diff --git a/src/ng/directive/ngView.js b/src/ng/directive/ngView.js index 2fd3a600..6e92c2d8 100644 --- a/src/ng/directive/ngView.js +++ b/src/ng/directive/ngView.js @@ -147,7 +147,7 @@ var ngViewDirective = ['$http', '$templateCache', '$route', '$anchorScroll', '$c if (current.controller) { locals.$scope = lastScope; controller = $controller(current.controller, locals); - element.contents().data('$ngControllerController', controller); + element.children().data('$ngControllerController', controller); } link(lastScope); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index da8f2f8c..6fbf99c3 100644 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -116,12 +116,17 @@ describe('$compile', function() { describe('compile phase', function() { + it('should attach scope to the document node when it is compiled explicitly', inject(function($document){ + $compile($document)($rootScope); + expect($document.scope()).toBe($rootScope); + })); + it('should wrap root text nodes in spans', inject(function($compile, $rootScope) { element = jqLite('<div>A<a>B</a>C</div>'); var text = element.contents(); expect(text[0].nodeName).toEqual('#text'); text = $compile(text)($rootScope); - expect(lowercase(text[0].nodeName)).toEqual('span'); + expect(text[0].nodeName).toEqual('SPAN'); expect(element.find('span').text()).toEqual('A<a>B</a>C'); })); @@ -137,6 +142,41 @@ describe('$compile', function() { expect(spans.text().indexOf('C')).toEqual(0); }); + it('should not leak memory when there are top level empty text nodes', function() { + var calcCacheSize = function() { + var size = 0; + forEach(jqLite.cache, function(item, key) { size++; }); + return size; + }; + + // We compile the contents of element (i.e. not element itself) + // Then delete these contents and check the cache has been reset to zero + + // First with only elements at the top level + element = jqLite('<div><div></div></div>'); + $compile(element.contents())($rootScope); + element.html(''); + expect(calcCacheSize()).toEqual(0); + + // Next with non-empty text nodes at the top level + // (in this case the compiler will wrap them in a <span>) + element = jqLite('<div>xxx</div>'); + $compile(element.contents())($rootScope); + element.html(''); + expect(calcCacheSize()).toEqual(0); + + // Next with comment nodes at the top level + element = jqLite('<div><!-- comment --></div>'); + $compile(element.contents())($rootScope); + element.html(''); + expect(calcCacheSize()).toEqual(0); + + // Finally with empty text nodes at the top level + element = jqLite('<div> \n<div></div> </div>'); + $compile(element.contents())($rootScope); + element.html(''); + expect(calcCacheSize()).toEqual(0); + }); describe('multiple directives per element', function() { it('should allow multiple directives per element', inject(function($compile, $rootScope, log){ diff --git a/test/ng/directive/ngViewSpec.js b/test/ng/directive/ngViewSpec.js index e90bb3bd..e781b98b 100644 --- a/test/ng/directive/ngViewSpec.js +++ b/test/ng/directive/ngViewSpec.js @@ -429,7 +429,7 @@ describe('ngView', function() { $rootScope.$digest(); expect($rootScope.load).toHaveBeenCalledOnce(); }); - }) + }); it('should set $scope and $controllerController on the view', function() { @@ -459,4 +459,28 @@ describe('ngView', function() { expect(div.controller()).toBe($route.current.scope.ctrl); }); }); + + it('should not set $scope or $controllerController on top level text elements in the view', function() { + function MyCtrl($scope) {} + + module(function($routeProvider) { + $routeProvider.when('/foo', {templateUrl: 'tpl.html', controller: MyCtrl}); + }); + + inject(function($templateCache, $location, $rootScope, $route) { + $templateCache.put('tpl.html', '<div></div> '); + $location.url('/foo'); + $rootScope.$digest(); + + forEach(element.contents(), function(node) { + if ( node.nodeType == 3 ) { + expect(jqLite(node).scope()).not.toBe($route.current.scope); + expect(jqLite(node).controller()).not.toBeDefined(); + } else { + expect(jqLite(node).scope()).toBe($route.current.scope); + expect(jqLite(node).controller()).toBeDefined(); + } + }); + }); + }); }); |
