From 9c0418cf1abd609bf0ffbe71fbdfa75905cf8e0f Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 4 May 2012 01:24:46 -0700 Subject: [PATCH] fix($compile): ignore ws when checking if template has single root Also add the same error checking for sync templates. Closes #910 --- src/ng/compile.js | 24 ++++++++---- test/ng/compileSpec.js | 85 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 91 insertions(+), 18 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 4a18b1a7edf..6a6ffc3c53a 100644 --- 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('
' + trim(directiveValue) + '
').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('
' + trim(content) + '
').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 index b2a6856b50b..c55928168c0 100644 --- 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: '
' + } + }); + $compileProvider.directive('singleRootWithWhiteSpace', function() { + return { + replace: true, + template: '
\n' + } + }); + }); + + inject(function($compile) { + expect(function() { + $compile('

'); + }).toThrow('Template must have exactly one root element. was: dada'); + + expect(function() { + $compile('

'); + }).toThrow('Template must have exactly one root element. was:
'); + + // ws is ok + expect(function() { + $compile('

'); + }).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 mid after'); - $compile('
'); - expect(function(){ - $httpBackend.flush(); - }).toThrow('Template must have exactly one root element: before mid after'); - })); - - it('should allow multiple elements in template', inject(function($compile, $httpBackend) { $httpBackend.expect('GET', 'hello.html').respond('before mid after'); element = jqLite('
'); @@ -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('

'); + $rootScope.$digest(); + expect($exceptionHandler.errors.pop().message). + toBe('Template must have exactly one root element. was: dada'); + + // multi root + $templateCache.put('template.html', '
'); + $compile('

'); + $rootScope.$digest(); + expect($exceptionHandler.errors.pop().message). + toBe('Template must have exactly one root element. was:
'); + + // ws is ok + $templateCache.put('template.html', '
\n'); + $compile('

'); + $rootScope.$apply(); + expect($exceptionHandler.errors).toEqual([]); + }); + }); });