From 79495ad51f7e17c0bbe84e48ce25ddcbf70c9012 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 21 Nov 2013 21:54:42 -0800 Subject: [PATCH] fix(ngInclude): Don't throw when the ngInclude element contains content with directives. Related to #5069 --- src/ng/directive/ngInclude.js | 29 +++++++++++++--------- test/ng/directive/ngIncludeSpec.js | 40 +++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/ng/directive/ngInclude.js b/src/ng/directive/ngInclude.js index 7f395f96407b..b721aa235fee 100644 --- a/src/ng/directive/ngInclude.js +++ b/src/ng/directive/ngInclude.js @@ -188,18 +188,23 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile' if (thisChangeId !== changeCounter) return; var newScope = scope.$new(); - $transclude(newScope, function(clone) { - cleanupLastIncludeContent(); - - currentScope = newScope; - currentElement = clone; - - currentElement.html(response); - $animate.enter(currentElement, null, $element, afterAnimation); - $compile(currentElement.contents())(currentScope); - currentScope.$emit('$includeContentLoaded'); - scope.$eval(onloadExp); - }); + // Note: This will also link all children of ng-include that were contained in the original + // html. If that content contains controllers, ... they could pollute/change the scope. + // However, using ng-include on an element with additional content does not make sense... + // Note: We can't remove them in the cloneAttchFn of $transclude as that + // function is called before linking the content, which would apply child + // directives to non existing elements. + var clone = $transclude(newScope, noop); + cleanupLastIncludeContent(); + + currentScope = newScope; + currentElement = clone; + + currentElement.html(response); + $animate.enter(currentElement, null, $element, afterAnimation); + $compile(currentElement.contents())(currentScope); + currentScope.$emit('$includeContentLoaded'); + scope.$eval(onloadExp); }).error(function() { if (thisChangeId === changeCounter) cleanupLastIncludeContent(); }); diff --git a/test/ng/directive/ngIncludeSpec.js b/test/ng/directive/ngIncludeSpec.js index 59f8b4aebc48..2d115e8b683f 100644 --- a/test/ng/directive/ngIncludeSpec.js +++ b/test/ng/directive/ngIncludeSpec.js @@ -465,10 +465,22 @@ describe('ngInclude', function() { }); describe('ngInclude and transcludes', function() { + var element, directive; + + beforeEach(module(function($compileProvider) { + element = null; + directive = $compileProvider.directive; + })); + + afterEach(function() { + if (element) { + dealoc(element); + } + }); + it('should allow access to directive controller from children when used in a replace template', function() { var controller; - module(function($compileProvider) { - var directive = $compileProvider.directive; + module(function() { directive('template', valueFn({ template: '
', replace: true, @@ -485,13 +497,33 @@ describe('ngInclude and transcludes', function() { }); inject(function($compile, $rootScope, $httpBackend) { $httpBackend.expectGET('include.html').respond('
'); - var element = $compile('
')($rootScope); + element = $compile('
')($rootScope); $rootScope.$apply(); $httpBackend.flush(); expect(controller.flag).toBe(true); - dealoc(element); }); }); + + it("should compile it's content correctly (although we remove it later)", function() { + var testElement; + module(function() { + directive('test', function() { + return { + link: function(scope, element) { + testElement = element; + } + }; + }); + }); + inject(function($compile, $rootScope, $httpBackend) { + $httpBackend.expectGET('include.html').respond(' '); + element = $compile('
')($rootScope); + $rootScope.$apply(); + $httpBackend.flush(); + expect(testElement[0].nodeName).toBe('DIV'); + }); + + }); }); describe('ngInclude animations', function() {