From e19067c9bbac3c3bb450c80f73eb5518bd0db1a1 Mon Sep 17 00:00:00 2001
From: Brian Ford <btford@umich.edu>
Date: Wed, 30 Oct 2013 14:51:02 -0700
Subject: [PATCH] fix(ngIf): ngIf removes elements dynamically added to it

When using ngIf with ngInclude on the same element, ngIf previously did not remove
elements added by ngInclude. Similarly, when using ngIfStart/End, ngIf will miss
elements added between the start/end markers added after ngIf is linked.

This commit changes the behavior of ngIf to add a comment node at the end of its
elements such that elements between the starting comment and this ending comment
are removed when ngIf's predicate does not hold.
---
 src/ng/compile.js             |  2 +-
 src/ng/directive/ngIf.js      | 35 +++++++++++++++++++++++++--------
 test/ng/directive/ngIfSpec.js | 37 +++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/src/ng/compile.js b/src/ng/compile.js
index 7754a8e6d69f..af7d5e6b319d 100644
--- a/src/ng/compile.js
+++ b/src/ng/compile.js
@@ -1175,7 +1175,7 @@ function $CompileProvider($provide) {
         if (directiveValue = directive.transclude) {
           // Special case ngRepeat so that we don't complain about duplicate transclusion, ngRepeat
           // knows how to handle this on its own.
-          if (directiveName !== 'ngRepeat') {
+          if (directiveName !== 'ngRepeat' && directiveName !== 'ngIf') {
             assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
             transcludeDirective = directive;
           }
diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js
index b2811ab423f1..662f3f320f72 100644
--- a/src/ng/directive/ngIf.js
+++ b/src/ng/directive/ngIf.js
@@ -86,20 +86,21 @@ var ngIfDirective = ['$animate', function($animate) {
     restrict: 'A',
     compile: function (element, attr, transclude) {
       return function ($scope, $element, $attr) {
-        var childElement, childScope;
+        var block = {}, childScope;
         $scope.$watch($attr.ngIf, function ngIfWatchAction(value) {
-          if (childElement) {
-            $animate.leave(childElement);
-            childElement = undefined;
+          if (block.startNode) {
+            $animate.leave(getBlockElements(block));
+            block = {};
           }
-          if (childScope) {
-            childScope.$destroy();
-            childScope = undefined;
+          if (block.startNode) {
+            getBlockElements(block).$destroy();
+            block = {};
           }
           if (toBoolean(value)) {
             childScope = $scope.$new();
             transclude(childScope, function (clone) {
-              childElement = clone;
+              block.startNode = clone[0];
+              block.endNode = clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ');
               $animate.enter(clone, $element.parent(), $element);
             });
           }
@@ -107,4 +108,22 @@ var ngIfDirective = ['$animate', function($animate) {
       };
     }
   };
+
+  // TODO(bford): this helper was copypasta'd from ngRepeat
+  function getBlockElements(block) {
+    if (block.startNode === block.endNode) {
+      return jqLite(block.startNode);
+    }
+
+    var element = block.startNode;
+    var elements = [element];
+
+    do {
+      element = element.nextSibling;
+      if (!element) break;
+      elements.push(element);
+    } while (element !== block.endNode);
+
+    return jqLite(elements);
+  }
 }];
diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js
index 79eab6bb2a11..509cf26db371 100755
--- a/test/ng/directive/ngIfSpec.js
+++ b/test/ng/directive/ngIfSpec.js
@@ -60,6 +60,43 @@ describe('ngIf', function () {
     expect(element.children().length).toBe(9);
   });
 
+  it('should play nice with ngInclude on the same element', inject(function($templateCache) {
+    $templateCache.put('test.html', [200, '{{value}}', {}]);
+
+    $scope.value = 'first';
+    element.append($compile(
+      '<div ng-if="value==\'first\'" ng-include="\'test.html\'"></div>'
+    )($scope));
+    $scope.$apply();
+    expect(element.text()).toBe('first');
+
+    $scope.value = 'later';
+    $scope.$apply();
+    expect(element.text()).toBe('');
+  }));
+
+  it('should work with multiple elements', function() {
+    $scope.show = true;
+    $scope.things = [1, 2, 3];
+    element.append($compile(
+      '<div>before;</div>' +
+        '<div ng-if-start="show">start;</div>' +
+        '<div ng-repeat="thing in things">{{thing}};</div>' +
+        '<div ng-if-end>end;</div>' +
+        '<div>after;</div>'
+    )($scope));
+    $scope.$apply();
+    expect(element.text()).toBe('before;start;1;2;3;end;after;');
+
+    $scope.things.push(4);
+    $scope.$apply();
+    expect(element.text()).toBe('before;start;1;2;3;4;end;after;');
+
+    $scope.show = false;
+    $scope.$apply();
+    expect(element.text()).toBe('before;after;');
+  });
+
   it('should restore the element to its compiled state', function() {
     $scope.value = true;
     makeIf('value');