fix($compile): don't throw tplrt error when there is a whitespace around a top-level comment

Prashant Singh Pawar · Narretz · commit 76d3dafdeaf2 · 2016-09-15T12:10:27.000+02:00
Added new conditional for NODE_TYPE_TEXT inside removeComments method of $compile Added corresponding unit tests. Closes angular#15108 PR (angular#15132)
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -3679,8 +3679,9 @@ function removeComments(jqNodes) {
 
   while (i--) {
     var node = jqNodes[i];
-    if (node.nodeType === NODE_TYPE_COMMENT) {
-      splice.call(jqNodes, i, 1);
+    if (node.nodeType === NODE_TYPE_COMMENT ||
+       (node.nodeType === NODE_TYPE_TEXT && node.nodeValue.trim() === '')) {
+         splice.call(jqNodes, i, 1);
     }
   }
   return jqNodes;
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -1364,6 +1364,22 @@ describe('$compile', function() {
           });
         });
 
+        it('should ignore whitespace betwee comment and root node when replacing with a template', function() {
+          module(function() {
+            directive('replaceWithWhitespace', valueFn({
+              replace: true,
+              template: '<!-- ignored comment --> <p>Hello, world!</p> <!-- ignored comment-->'
+            }));
+          });
+          inject(function($compile, $rootScope) {
+            expect(function() {
+              element = $compile('<div><div replace-with-whitespace></div></div>')($rootScope);
+            }).not.toThrow();
+            expect(element.find('p').length).toBe(1);
+            expect(element.find('p').text()).toBe('Hello, world!');
+          });
+        });
+
         it('should keep prototype properties on directive', function() {
           module(function() {
             function DirectiveClass() {
@@ -2092,6 +2108,18 @@ describe('$compile', function() {
             $compile('<p template></p>');
             $rootScope.$apply();
             expect($exceptionHandler.errors).toEqual([]);
+
+            // comments are ok
+            $templateCache.put('template.html', '<!-- oh hi --><div></div> \n');
+            $compile('<p template></p>');
+            $rootScope.$apply();
+            expect($exceptionHandler.errors).toEqual([]);
+
+            // white space around comments is ok
+            $templateCache.put('template.html', '  <!-- oh hi -->  <div></div>  <!-- oh hi -->\n');
+            $compile('<p template></p>');
+            $rootScope.$apply();
+            expect($exceptionHandler.errors).toEqual([]);
           });
         });
 
@@ -2303,6 +2331,26 @@ describe('$compile', function() {
           });
         });
 
+        it('should ignore whitespace between comment and root node when replacing with a templateUrl', function() {
+          module(function() {
+            directive('replaceWithWhitespace', valueFn({
+              replace: true,
+              templateUrl: 'templateWithWhitespace.html'
+            }));
+          });
+          inject(function($compile, $rootScope, $httpBackend) {
+            $httpBackend.whenGET('templateWithWhitespace.html').
+              respond('<!-- ignored comment --> <p>Hello, world!</p> <!-- ignored comment-->');
+            expect(function() {
+              element = $compile('<div><div replace-with-whitespace></div></div>')($rootScope);
+            }).not.toThrow();
+            $httpBackend.flush();
+            expect(element.find('p').length).toBe(1);
+            expect(element.find('p').text()).toBe('Hello, world!');
+          });
+        });
+
+
         it('should keep prototype properties on sync version of async directive', function() {
           module(function() {
             function DirectiveClass() {

Original file line number	Diff line number	Diff line change
`@@ -3679,8 +3679,9 @@ function removeComments(jqNodes) {`
`3679`	`3679`
`3680`	`3680`	`while (i--) {`
`3681`	`3681`	`var node = jqNodes[i];`
`3682`		`- if (node.nodeType === NODE_TYPE_COMMENT) {`
`3683`		`- splice.call(jqNodes, i, 1);`
	`3682`	`+ if (node.nodeType === NODE_TYPE_COMMENT \|\|`
	`3683`	`+ (node.nodeType === NODE_TYPE_TEXT && node.nodeValue.trim() === '')) {`
	`3684`	`+ splice.call(jqNodes, i, 1);`
`3684`	`3685`	`}`
`3685`	`3686`	`}`
`3686`	`3687`	`return jqNodes;`