diff options
| author | Tobias Bosch | 2013-11-26 19:55:02 -0800 | 
|---|---|---|
| committer | Tobias Bosch | 2013-12-05 22:16:25 -0800 | 
| commit | b0972a2e75909e41dbac6e4413ada7df2d51df3a (patch) | |
| tree | daefd834b62abcbe3ff970e2942cca16f67c491d | |
| parent | 2dbb6f9a54eb5ff5847eed11c85ac4cf119eb41c (diff) | |
| download | angular.js-b0972a2e75909e41dbac6e4413ada7df2d51df3a.tar.bz2 | |
fix($compile): update cloned elements if the template arrives after the cloning
If an element has a directive whose content is loaded using `templateUrl`,
and the element is cloned using a linking function before the template arrives,
the clone needs to be updated as well.
This also updates `ngIf` and `ngRepeat` to keep the connection to the clone
of a tranclude function, so that they know about the changes a directive with
`templateUrl` does to the element in the future.
Fixes to #4930.
| -rw-r--r-- | src/Angular.js | 16 | ||||
| -rw-r--r-- | src/ng/compile.js | 2 | ||||
| -rw-r--r-- | src/ng/directive/ngIf.js | 9 | ||||
| -rw-r--r-- | src/ng/directive/ngRepeat.js | 30 | ||||
| -rwxr-xr-x | test/ng/compileSpec.js | 53 | ||||
| -rwxr-xr-x | test/ng/directive/ngIfSpec.js | 27 | ||||
| -rw-r--r-- | test/ng/directive/ngRepeatSpec.js | 24 | 
7 files changed, 138 insertions, 23 deletions
| diff --git a/src/Angular.js b/src/Angular.js index 478ef2a2..b09d3a7f 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -1330,23 +1330,25 @@ function getter(obj, path, bindFnToScope) {  }  /** - * Return the siblings between `startNode` and `endNode`, inclusive - * @param {Object} object with `startNode` and `endNode` properties + * Return the DOM siblings between the first and last node in the given array. + * @param {Array} array like object   * @returns jQlite object containing the elements   */ -function getBlockElements(block) { -  if (block.startNode === block.endNode) { -    return jqLite(block.startNode); +function getBlockElements(nodes) { +  var startNode = nodes[0], +      endNode = nodes[nodes.length - 1]; +  if (startNode === endNode) { +    return jqLite(startNode);    } -  var element = block.startNode; +  var element = startNode;    var elements = [element];    do {      element = element.nextSibling;      if (!element) break;      elements.push(element); -  } while (element !== block.endNode); +  } while (element !== endNode);    return jqLite(elements);  } diff --git a/src/ng/compile.js b/src/ng/compile.js index fd8a8729..7d0bb008 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -931,7 +931,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {                  createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn)                );              } else { -              nodeLinkFn(childLinkFn, childScope, node, undefined, boundTranscludeFn); +              nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn);              }            } else if (childLinkFn) {              childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn); diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js index dcb3825d..a05d30d0 100644 --- a/src/ng/directive/ngIf.js +++ b/src/ng/directive/ngIf.js @@ -94,9 +94,12 @@ var ngIfDirective = ['$animate', function($animate) {              if (!childScope) {                childScope = $scope.$new();                $transclude(childScope, function (clone) { +                clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' '); +                // Note: We only need the first/last node of the cloned nodes. +                // However, we need to keep the reference to the jqlite wrapper as it might be changed later +                // by a directive with templateUrl when it's template arrives.                  block = { -                  startNode: clone[0], -                  endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ') +                  clone: clone                  };                  $animate.enter(clone, $element.parent(), $element);                }); @@ -109,7 +112,7 @@ var ngIfDirective = ['$animate', function($animate) {              }              if (block) { -              $animate.leave(getBlockElements(block)); +              $animate.leave(getBlockElements(block.clone));                block = null;              }            } diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index cd605bf3..86874a41 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -301,7 +301,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {             } else if (nextBlockMap.hasOwnProperty(trackById)) {               // restore lastBlockMap               forEach(nextBlockOrder, function(block) { -               if (block && block.startNode) lastBlockMap[block.id] = block; +               if (block && block.scope) lastBlockMap[block.id] = block;               });               // This is a duplicate and we need to throw an error               throw ngRepeatMinErr('dupes', "Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}", @@ -318,7 +318,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {              // lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn              if (lastBlockMap.hasOwnProperty(key)) {                block = lastBlockMap[key]; -              elementsToRemove = getBlockElements(block); +              elementsToRemove = getBlockElements(block.clone);                $animate.leave(elementsToRemove);                forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; });                block.scope.$destroy(); @@ -330,9 +330,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {              key = (collection === collectionKeys) ? index : collectionKeys[index];              value = collection[key];              block = nextBlockOrder[index]; -            if (nextBlockOrder[index - 1]) previousNode = nextBlockOrder[index - 1].endNode; +            if (nextBlockOrder[index - 1]) previousNode = getBlockEnd(nextBlockOrder[index - 1]); -            if (block.startNode) { +            if (block.scope) {                // if we have already seen this object, then we need to reuse the                // associated scope/element                childScope = block.scope; @@ -342,11 +342,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {                  nextNode = nextNode.nextSibling;                } while(nextNode && nextNode[NG_REMOVED]); -              if (block.startNode != nextNode) { +              if (getBlockStart(block) != nextNode) {                  // existing item which got moved -                $animate.move(getBlockElements(block), null, jqLite(previousNode)); +                $animate.move(getBlockElements(block.clone), null, jqLite(previousNode));                } -              previousNode = block.endNode; +              previousNode = getBlockEnd(block);              } else {                // new item which we don't know about                childScope = $scope.$new(); @@ -362,14 +362,16 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {              childScope.$odd = !(childScope.$even = (index&1) === 0);              // jshint bitwise: true -            if (!block.startNode) { +            if (!block.scope) {                $transclude(childScope, function(clone) {                  clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');                  $animate.enter(clone, null, jqLite(previousNode));                  previousNode = clone;                  block.scope = childScope; -                block.startNode = previousNode && previousNode.endNode ? previousNode.endNode : clone[0]; -                block.endNode = clone[clone.length - 1]; +                // Note: We only need the first/last node of the cloned nodes. +                // However, we need to keep the reference to the jqlite wrapper as it might be changed later +                // by a directive with templateUrl when it's template arrives. +                block.clone = clone;                  nextBlockMap[block.id] = block;                });              } @@ -378,5 +380,13 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {          });      }    }; + +  function getBlockStart(block) { +    return block.clone[0]; +  } + +  function getBlockEnd(block) { +    return block.clone[block.clone.length - 1]; +  }  }]; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index c017bfa6..f2fa4ef6 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -969,6 +969,32 @@ describe('$compile', function() {            });          }); +        it('should resolve widgets after cloning in append mode without $templateCache', function() { +          module(function($exceptionHandlerProvider) { +            $exceptionHandlerProvider.mode('log'); +          }); +          inject(function($compile, $templateCache, $rootScope, $httpBackend, $browser, +                          $exceptionHandler) { +            $httpBackend.expect('GET', 'cau.html').respond('<span>{{name}}</span>'); +            $rootScope.name = 'Elvis'; +            var template = $compile('<div class="cau"></div>'); +            var e1; +            var e2; + +            e1 = template($rootScope.$new(), noop); // clone +            expect(e1.text()).toEqual(''); + +            $httpBackend.flush(); + +            e2 = template($rootScope.$new(), noop); // clone +            $rootScope.$digest(); +            expect(e1.text()).toEqual('Elvis'); +            expect(e2.text()).toEqual('Elvis'); + +            dealoc(e1); +            dealoc(e2); +          }); +        });          it('should resolve widgets after cloning in inline mode', function() {            module(function($exceptionHandlerProvider) { @@ -1010,6 +1036,33 @@ describe('$compile', function() {            });          }); +        it('should resolve widgets after cloning in inline mode without $templateCache', function() { +          module(function($exceptionHandlerProvider) { +            $exceptionHandlerProvider.mode('log'); +          }); +          inject(function($compile, $templateCache, $rootScope, $httpBackend, $browser, +                          $exceptionHandler) { +            $httpBackend.expect('GET', 'cau.html').respond('<span>{{name}}</span>'); +            $rootScope.name = 'Elvis'; +            var template = $compile('<div class="i-cau"></div>'); +            var e1; +            var e2; + +            e1 = template($rootScope.$new(), noop); // clone +            expect(e1.text()).toEqual(''); + +            $httpBackend.flush(); + +            e2 = template($rootScope.$new(), noop); // clone +            $rootScope.$digest(); +            expect(e1.text()).toEqual('Elvis'); +            expect(e2.text()).toEqual('Elvis'); + +            dealoc(e1); +            dealoc(e2); +          }); +        }); +          it('should be implicitly terminal and not compile placeholder content in append', inject(              function($compile, $templateCache, $rootScope, log) { diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js index 427bfd59..db923150 100755 --- a/test/ng/directive/ngIfSpec.js +++ b/test/ng/directive/ngIfSpec.js @@ -1,8 +1,11 @@  'use strict';  describe('ngIf', function () { -  var $scope, $compile, element; +  var $scope, $compile, element, $compileProvider; +  beforeEach(module(function(_$compileProvider_) { +    $compileProvider = _$compileProvider_; +  }));    beforeEach(inject(function ($rootScope, _$compile_) {      $scope = $rootScope.$new();      $compile = _$compile_; @@ -146,6 +149,28 @@ describe('ngIf', function () {      expect(element.children()[0].className).toContain('my-class');    }); +  it('should work when combined with an ASYNC template that loads after the first digest', inject(function($httpBackend, $compile, $rootScope) { +    $compileProvider.directive('test', function() { +      return { +        templateUrl: 'test.html' +      }; +    }); +    $httpBackend.whenGET('test.html').respond('hello'); +    element.append('<div ng-if="show" test></div>'); +    $compile(element)($rootScope); +    $rootScope.show = true; +    $rootScope.$apply(); +    expect(element.text()).toBe(''); + +    $httpBackend.flush(); +    expect(element.text()).toBe('hello'); + +    $rootScope.show = false; +    $rootScope.$apply(); +    // Note: there are still comments in element! +    expect(element.children().length).toBe(0); +    expect(element.text()).toBe(''); +  }));  });  describe('ngIf and transcludes', function() { diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 6584f31a..bdc0b8f5 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -749,8 +749,30 @@ describe('ngRepeat', function() {          expect(element.text()).toBe('123');        }));      } -  }); +    it('should work when combined with an ASYNC template that loads after the first digest', inject(function($httpBackend, $compile, $rootScope) { +      $compileProvider.directive('test', function() { +        return { +          templateUrl: 'test.html' +        }; +      }); +      $httpBackend.whenGET('test.html').respond('hello'); +      element = jqLite('<div><div ng-repeat="i in items" test></div></div>'); +      $compile(element)($rootScope); +      $rootScope.items = [1]; +      $rootScope.$apply(); +      expect(element.text()).toBe(''); + +      $httpBackend.flush(); +      expect(element.text()).toBe('hello'); + +      $rootScope.items = []; +      $rootScope.$apply(); +      // Note: there are still comments in element! +      expect(element.children().length).toBe(0); +      expect(element.text()).toBe(''); +    })); +  });    it('should add separator comments after each item', inject(function ($compile, $rootScope) {      var check = function () { | 
