fix($compile): ignore ws when checking if template has single root

Also add the same error checking for sync templates. Closes #910
angular · May 4, 2012 · 9c0418c · 9c0418c
1 parent 1564b82
commit 9c0418c
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 18 deletions.
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -145,7 +145,7 @@ function $CompileProvider($provide) {
       Suffix = 'Directive',
       COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
       CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
-      HAS_ROOT_ELEMENT = /^\<[\s\S]*\>$/;
+      MULTI_ROOT_TEMPLATE_ERROR = 'Template must have exactly one root element. was: ';
 
 
   this.directive = function registerDirective(name, directiveFactory) {
@@ -587,8 +587,14 @@ function $CompileProvider($provide) {
           assertNoDuplicate('template', templateDirective, directive, $compileNode);
           templateDirective = directive;
 
-          compileNode = jqLite(directiveValue)[0];
+          $template = jqLite('<div>' + trim(directiveValue) + '</div>').contents();
+          compileNode = $template[0];
+
           if (directive.replace) {
+            if ($template.length != 1 || compileNode.nodeType !== 1) {
+              throw new Error(MULTI_ROOT_TEMPLATE_ERROR + directiveValue);
+            }
+
             replaceWith($rootElement, $compileNode, compileNode);
 
             var newTemplateAttrs = {$attr: {}};
@@ -840,15 +846,17 @@ function $CompileProvider($provide) {
 
       $http.get(origAsyncDirective.templateUrl, {cache: $templateCache}).
         success(function(content) {
-          if (replace && !content.match(HAS_ROOT_ELEMENT)) {
-            throw Error('Template must have exactly one root element: ' + content);
-          }
-
-          var compileNode, tempTemplateAttrs;
+          var compileNode, tempTemplateAttrs, $template;
 
           if (replace) {
+            $template = jqLite('<div>' + trim(content) + '</div>').contents();
+            compileNode = $template[0];
+
+            if ($template.length != 1 || compileNode.nodeType !== 1) {
+              throw new Error(MULTI_ROOT_TEMPLATE_ERROR + content);
+            }
+
             tempTemplateAttrs = {$attr: {}};
-            compileNode = jqLite(content)[0];
             replaceWith($rootElement, $compileNode, compileNode);
             collectDirectives(compileNode, directives, tempTemplateAttrs);
             mergeTemplateAttributes(tAttrs, tempTemplateAttrs);

diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -487,10 +487,48 @@ describe('$compile', function() {
           expect(child).toHaveClass('three');
           expect(child).toHaveClass('log'); // merged from replace directive template
         }));
+
+        it("should fail if replacing and template doesn't have a single root element", function() {
+          module(function($compileProvider) {
+            $compileProvider.directive('noRootElem', function() {
+              return {
+                replace: true,
+                template: 'dada'
+              }
+            });
+            $compileProvider.directive('multiRootElem', function() {
+              return {
+                replace: true,
+                template: '<div></div><div></div>'
+              }
+            });
+            $compileProvider.directive('singleRootWithWhiteSpace', function() {
+              return {
+                replace: true,
+                template: '  <div></div> \n'
+              }
+            });
+          });
+
+          inject(function($compile) {
+            expect(function() {
+              $compile('<p no-root-elem></p>');
+            }).toThrow('Template must have exactly one root element. was: dada');
+
+            expect(function() {
+              $compile('<p multi-root-elem></p>');
+            }).toThrow('Template must have exactly one root element. was: <div></div><div></div>');
+
+            // ws is ok
+            expect(function() {
+              $compile('<p single-root-with-white-space></p>');
+            }).not.toThrow();
+          });
+        });
       });
 
 
-      describe('async templates', function() {
+      describe('templateUrl', function() {
 
         beforeEach(module(
           function($compileProvider) {
@@ -916,15 +954,6 @@ describe('$compile', function() {
         });
 
 
-        it('should check that template has root element', inject(function($compile, $httpBackend) {
-          $httpBackend.expect('GET', 'hello.html').respond('before <b>mid</b> after');
-          $compile('<div i-hello></div>');
-          expect(function(){
-            $httpBackend.flush();
-          }).toThrow('Template must have exactly one root element: before <b>mid</b> after');
-        }));
-
-
         it('should allow multiple elements in template', inject(function($compile, $httpBackend) {
           $httpBackend.expect('GET', 'hello.html').respond('before <b>mid</b> after');
           element = jqLite('<div hello></div>');
@@ -958,6 +987,42 @@ describe('$compile', function() {
             expect(element.text()).toEqual('i=1;i=2;');
           }
         ));
+
+
+        it("should fail if replacing and template doesn't have a single root element", function() {
+          module(function($exceptionHandlerProvider, $compileProvider) {
+            $exceptionHandlerProvider.mode('log');
+
+            $compileProvider.directive('template', function() {
+              return {
+                replace: true,
+                templateUrl: 'template.html'
+              }
+            });
+          });
+
+          inject(function($compile, $templateCache, $rootScope, $exceptionHandler) {
+            // no root element
+            $templateCache.put('template.html', 'dada');
+            $compile('<p template></p>');
+            $rootScope.$digest();
+            expect($exceptionHandler.errors.pop().message).
+                toBe('Template must have exactly one root element. was: dada');
+
+            // multi root
+            $templateCache.put('template.html', '<div></div><div></div>');
+            $compile('<p template></p>');
+            $rootScope.$digest();
+            expect($exceptionHandler.errors.pop().message).
+                toBe('Template must have exactly one root element. was: <div></div><div></div>');
+
+            // ws is ok
+            $templateCache.put('template.html', '  <div></div> \n');
+            $compile('<p template></p>');
+            $rootScope.$apply();
+            expect($exceptionHandler.errors).toEqual([]);
+          });
+        });
       });