From a050d1793ea3ec4e7df4882f9c34ea57830e85cf Mon Sep 17 00:00:00 2001
From: Peter Bacon Darwin <pete@bacondarwin.com>
Date: Thu, 16 Jun 2016 13:46:58 +0100
Subject: [PATCH] fix(ngTransclude): ensure that fallback content is compiled
 and linked correctly

Closes #14787
---
 src/ng/directive/ngTransclude.js |  75 +++++++++++-------
 test/ng/compileSpec.js           | 129 +++++++++++++++++++++----------
 2 files changed, 132 insertions(+), 72 deletions(-)

diff --git a/src/ng/directive/ngTransclude.js b/src/ng/directive/ngTransclude.js
index 5b31b06a2248..6b02963f16d9 100644
--- a/src/ng/directive/ngTransclude.js
+++ b/src/ng/directive/ngTransclude.js
@@ -163,41 +163,56 @@ var ngTranscludeDirective = ['$compile', function($compile) {
   return {
     restrict: 'EAC',
     terminal: true,
-    link: function($scope, $element, $attrs, controller, $transclude) {
-      if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
-        // If the attribute is of the form: `ng-transclude="ng-transclude"`
-        // then treat it like the default
-        $attrs.ngTransclude = '';
-      }
+    compile: function ngTranscludeCompile(tElement) {
+
+      // Remove and cache any original content to act as a fallback
+      var fallbackLinkFn = $compile(tElement.contents());
+      tElement.empty();
+
+      return function ngTranscludePostLink($scope, $element, $attrs, controller, $transclude) {
+
+        if (!$transclude) {
+          throw ngTranscludeMinErr('orphan',
+          'Illegal use of ngTransclude directive in the template! ' +
+          'No parent directive that requires a transclusion found. ' +
+          'Element: {0}',
+          startingTag($element));
+        }
 
-      function ngTranscludeCloneAttachFn(clone, transcludedScope) {
-        if (clone.length) {
-          $element.empty();
-          $element.append(clone);
-        } else {
-          // Since this is the fallback content rather than the transcluded content,
-          // we compile against the scope we were linked against rather than the transcluded
-          // scope since this is the directive's own content
-          $compile($element.contents())($scope);
 
-          // There is nothing linked against the transcluded scope since no content was available,
-          // so it should be safe to clean up the generated scope.
-          transcludedScope.$destroy();
+        // If the attribute is of the form: `ng-transclude="ng-transclude"` then treat it like the default
+        if ($attrs.ngTransclude === $attrs.$attr.ngTransclude) {
+          $attrs.ngTransclude = '';
         }
-      }
+        var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
 
-      if (!$transclude) {
-        throw ngTranscludeMinErr('orphan',
-        'Illegal use of ngTransclude directive in the template! ' +
-        'No parent directive that requires a transclusion found. ' +
-        'Element: {0}',
-        startingTag($element));
-      }
+        // If the slot is required and no transclusion content is provided then this call will throw an error
+        $transclude(ngTranscludeCloneAttachFn, null, slotName);
 
-      // If there is no slot name defined or the slot name is not optional
-      // then transclude the slot
-      var slotName = $attrs.ngTransclude || $attrs.ngTranscludeSlot;
-      $transclude(ngTranscludeCloneAttachFn, null, slotName);
+        // If the slot is optional and no transclusion content is provided then use the fallback content
+        if (slotName && !$transclude.isSlotFilled(slotName)) {
+          useFallbackContent();
+        }
+
+        function ngTranscludeCloneAttachFn(clone, transcludedScope) {
+          if (clone.length) {
+            $element.append(clone);
+          } else {
+            useFallbackContent();
+            // There is nothing linked against the transcluded scope since no content was available,
+            // so it should be safe to clean up the generated scope.
+            transcludedScope.$destroy();
+          }
+        }
+
+        function useFallbackContent() {
+          // Since this is the fallback content rather than the transcluded content,
+          // we link against the scope of this directive rather than the transcluded scope
+          fallbackLinkFn($scope, function(clone) {
+            $element.append(clone);
+          });
+        }
+      };
     }
   };
 }];
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
index d1ef1d3b2f3a..e8d9d277ea99 100755
--- a/test/ng/compileSpec.js
+++ b/test/ng/compileSpec.js
@@ -7970,17 +7970,52 @@ describe('$compile', function() {
       });
 
 
-      it('should not compile the fallback content if transcluded content is provided', function() {
-        var contentsDidLink = false;
+      it('should clear the fallback content from the element during compile and before linking', function() {
+        module(function() {
+          directive('trans', function() {
+            return {
+              transclude: true,
+              template: '<div ng-transclude>fallback content</div>'
+            };
+          });
+        });
+        inject(function(log, $rootScope, $compile) {
+          element = jqLite('<div trans></div>');
+          var linkfn = $compile(element);
+          expect(element.html()).toEqual('<div ng-transclude=""></div>');
+          linkfn($rootScope);
+          $rootScope.$apply();
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">fallback content</div>');
+        });
+      });
+
+
+      it('should allow cloning of the fallback via ngRepeat', function() {
+        module(function() {
+          directive('trans', function() {
+            return {
+              transclude: true,
+              template: '<div ng-repeat="i in [0,1,2]"><div ng-transclude>{{i}}</div></div>'
+            };
+          });
+        });
+        inject(function(log, $rootScope, $compile) {
+          element = $compile('<div trans></div>')($rootScope);
+          $rootScope.$apply();
+          expect(element.text()).toEqual('012');
+        });
+      });
+
+
+      it('should not link the fallback content if transcluded content is provided', function() {
+        var linkSpy = jasmine.createSpy('postlink');
 
         module(function() {
           directive('inner', function() {
             return {
               restrict: 'E',
               template: 'old stuff! ',
-              link: function() {
-                contentsDidLink = true;
-              }
+              link: linkSpy
             };
           });
 
@@ -7995,21 +8030,19 @@ describe('$compile', function() {
           element = $compile('<div trans>unicorn!</div>')($rootScope);
           $rootScope.$apply();
           expect(sortedHtml(element.html())).toEqual('<div ng-transclude="">unicorn!</div>');
-          expect(contentsDidLink).toBe(false);
+          expect(linkSpy).not.toHaveBeenCalled();
         });
       });
 
       it('should compile and link the fallback content if no transcluded content is provided', function() {
-        var contentsDidLink = false;
+        var linkSpy = jasmine.createSpy('postlink');
 
         module(function() {
           directive('inner', function() {
             return {
               restrict: 'E',
               template: 'old stuff! ',
-              link: function() {
-                contentsDidLink = true;
-              }
+              link: linkSpy
             };
           });
 
@@ -8024,7 +8057,50 @@ describe('$compile', function() {
           element = $compile('<div trans></div>')($rootScope);
           $rootScope.$apply();
           expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""><inner>old stuff! </inner></div>');
-          expect(contentsDidLink).toBe(true);
+          expect(linkSpy).toHaveBeenCalled();
+        });
+      });
+
+      it('should compile and link the fallback content if an optional transclusion slot is not provided', function() {
+        var linkSpy = jasmine.createSpy('postlink');
+
+        module(function() {
+          directive('inner', function() {
+            return {
+              restrict: 'E',
+              template: 'old stuff! ',
+              link: linkSpy
+            };
+          });
+
+          directive('trans', function() {
+            return {
+              transclude: { optionalSlot: '?optional'},
+              template: '<div ng-transclude="optionalSlot"><inner></inner></div>'
+            };
+          });
+        });
+        inject(function(log, $rootScope, $compile) {
+          element = $compile('<div trans></div>')($rootScope);
+          $rootScope.$apply();
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude="optionalSlot"><inner>old stuff! </inner></div>');
+          expect(linkSpy).toHaveBeenCalled();
+        });
+      });
+
+      it('should cope if there is neither transcluded content nor fallback content', function() {
+        module(function() {
+          directive('trans', function() {
+            return {
+              transclude: true,
+              template: '<div ng-transclude></div>'
+            };
+          });
+        });
+        inject(function($rootScope, $compile) {
+          element = $compile('<div trans></div>')($rootScope);
+          $rootScope.$apply();
+          expect(sortedHtml(element.html())).toEqual('<div ng-transclude=""></div>');
         });
       });
 
@@ -9824,37 +9900,6 @@ describe('$compile', function() {
         expect(element.children().eq(2).text()).toEqual('dorothy');
       });
     });
-
-    it('should not overwrite the contents of an `ng-transclude` element, if the matching optional slot is not filled', function() {
-      module(function() {
-        directive('minionComponent', function() {
-          return {
-            restrict: 'E',
-            scope: {},
-            transclude: {
-              minionSlot: 'minion',
-              bossSlot: '?boss'
-            },
-            template:
-              '<div class="boss" ng-transclude="bossSlot">default boss content</div>' +
-              '<div class="minion" ng-transclude="minionSlot">default minion content</div>' +
-              '<div class="other" ng-transclude>default content</div>'
-          };
-        });
-      });
-      inject(function($rootScope, $compile) {
-        element = $compile(
-          '<minion-component>' +
-            '<minion>stuart</minion>' +
-            '<span>dorothy</span>' +
-            '<minion>kevin</minion>' +
-          '</minion-component>')($rootScope);
-        $rootScope.$apply();
-        expect(element.children().eq(0).text()).toEqual('default boss content');
-        expect(element.children().eq(1).text()).toEqual('stuartkevin');
-        expect(element.children().eq(2).text()).toEqual('dorothy');
-      });
-    });
   });