fix(ngView): Don't throw when the ngView element contains content wit…

…h directives. Fixes angular#5069
jamesdaily · Jan 27, 2014 · d314eee · d314eee
1 parent 462d709
commit d314eee
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 34 deletions.
diff --git a/src/ngRoute/directive/ngView.js b/src/ngRoute/directive/ngView.js
@@ -199,37 +199,43 @@ function ngViewFactory(   $route,   $anchorScroll,   $compile,   $controller,
 
           if (template) {
             var newScope = scope.$new();
-            $transclude(newScope, function(clone) {
-              clone.html(template);
-              $animate.enter(clone, null, currentElement || $element, function onNgViewEnter () {
-                if (angular.isDefined(autoScrollExp)
-                  && (!autoScrollExp || scope.$eval(autoScrollExp))) {
-                  $anchorScroll();
-                }
-              });
-
-              cleanupLastView();
-
-              var link = $compile(clone.contents()),
-                  current = $route.current;
-
-              currentScope = current.scope = newScope;
-              currentElement = clone;
-
-              if (current.controller) {
-                locals.$scope = currentScope;
-                var controller = $controller(current.controller, locals);
-                if (current.controllerAs) {
-                  currentScope[current.controllerAs] = controller;
-                }
-                clone.data('$ngControllerController', controller);
-                clone.children().data('$ngControllerController', controller);
-              }
 
-              link(currentScope);
-              currentScope.$emit('$viewContentLoaded');
-              currentScope.$eval(onloadExp);
+            // Note: This will also link all children of ng-view that were contained in the original
+            // html. If that content contains controllers, ... they could pollute/change the scope.
+            // However, using ng-view 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, angular.noop);
+            clone.html(template);
+            $animate.enter(clone, null, currentElement || $element, function onNgViewEnter () {
+              if (angular.isDefined(autoScrollExp)
+                && (!autoScrollExp || scope.$eval(autoScrollExp))) {
+                $anchorScroll();
+              }
             });
+
+            cleanupLastView();
+
+            var link = $compile(clone.contents()),
+                current = $route.current;
+
+            currentScope = current.scope = newScope;
+            currentElement = clone;
+
+            if (current.controller) {
+              locals.$scope = currentScope;
+              var controller = $controller(current.controller, locals);
+              if (current.controllerAs) {
+                currentScope[current.controllerAs] = controller;
+              }
+              clone.data('$ngControllerController', controller);
+              clone.children().data('$ngControllerController', controller);
+            }
+
+            link(currentScope);
+            currentScope.$emit('$viewContentLoaded');
+            currentScope.$eval(onloadExp);
           } else {
             cleanupLastView();
           }

diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js
@@ -515,12 +515,23 @@ describe('ngView', function() {
 });
 
 describe('ngView and transcludes', function() {
+  var element, directive;
+
+  beforeEach(module('ngRoute', 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('ngRoute');
-    module(function($compileProvider, $routeProvider) {
+    module(function($routeProvider) {
       $routeProvider.when('/view', {templateUrl: 'view.html'});
-      var directive = $compileProvider.directive;
       directive('template', function() {
         return {
           template: '<div ng-view></div>',
@@ -542,14 +553,35 @@ describe('ngView and transcludes', function() {
     });
     inject(function($compile, $rootScope, $httpBackend, $location) {
       $httpBackend.expectGET('view.html').respond('<div><div test></div></div>');
-      var element = $compile('<div><div template></div></div>')($rootScope);
+      element = $compile('<div><div template></div></div>')($rootScope);
       $location.url('/view');
       $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($compileProvider, $routeProvider) {
+      $routeProvider.when('/view', {template: ' '});
+      var directive = $compileProvider.directive;
+      directive('test', function() {
+        return {
+          link: function(scope, element) {
+            testElement = element;
+          }
+        };
+      });
+    });
+    inject(function($compile, $rootScope, $location) {
+      element = $compile('<div><div ng-view><div test someAttr></div></div></div>')($rootScope);
+      $location.url('/view');
+      $rootScope.$apply();
+      expect(testElement[0].nodeName).toBe('DIV');
+    });
+
+  });
 });
 
 describe('ngView animations', function() {