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.
This commit is contained in:
Tobias Bosch 2013-11-26 19:55:02 -08:00
parent 2dbb6f9a54
commit b0972a2e75
7 changed files with 138 additions and 23 deletions

View file

@ -1330,23 +1330,25 @@ function getter(obj, path, bindFnToScope) {
} }
/** /**
* Return the siblings between `startNode` and `endNode`, inclusive * Return the DOM siblings between the first and last node in the given array.
* @param {Object} object with `startNode` and `endNode` properties * @param {Array} array like object
* @returns jQlite object containing the elements * @returns jQlite object containing the elements
*/ */
function getBlockElements(block) { function getBlockElements(nodes) {
if (block.startNode === block.endNode) { var startNode = nodes[0],
return jqLite(block.startNode); endNode = nodes[nodes.length - 1];
if (startNode === endNode) {
return jqLite(startNode);
} }
var element = block.startNode; var element = startNode;
var elements = [element]; var elements = [element];
do { do {
element = element.nextSibling; element = element.nextSibling;
if (!element) break; if (!element) break;
elements.push(element); elements.push(element);
} while (element !== block.endNode); } while (element !== endNode);
return jqLite(elements); return jqLite(elements);
} }

View file

@ -931,7 +931,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn) createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn)
); );
} else { } else {
nodeLinkFn(childLinkFn, childScope, node, undefined, boundTranscludeFn); nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn);
} }
} else if (childLinkFn) { } else if (childLinkFn) {
childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn); childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn);

View file

@ -94,9 +94,12 @@ var ngIfDirective = ['$animate', function($animate) {
if (!childScope) { if (!childScope) {
childScope = $scope.$new(); childScope = $scope.$new();
$transclude(childScope, function (clone) { $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 = { block = {
startNode: clone[0], clone: clone
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
}; };
$animate.enter(clone, $element.parent(), $element); $animate.enter(clone, $element.parent(), $element);
}); });
@ -109,7 +112,7 @@ var ngIfDirective = ['$animate', function($animate) {
} }
if (block) { if (block) {
$animate.leave(getBlockElements(block)); $animate.leave(getBlockElements(block.clone));
block = null; block = null;
} }
} }

View file

@ -301,7 +301,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
} else if (nextBlockMap.hasOwnProperty(trackById)) { } else if (nextBlockMap.hasOwnProperty(trackById)) {
// restore lastBlockMap // restore lastBlockMap
forEach(nextBlockOrder, function(block) { 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 // 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}", 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 // lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn
if (lastBlockMap.hasOwnProperty(key)) { if (lastBlockMap.hasOwnProperty(key)) {
block = lastBlockMap[key]; block = lastBlockMap[key];
elementsToRemove = getBlockElements(block); elementsToRemove = getBlockElements(block.clone);
$animate.leave(elementsToRemove); $animate.leave(elementsToRemove);
forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; }); forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; });
block.scope.$destroy(); block.scope.$destroy();
@ -330,9 +330,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
key = (collection === collectionKeys) ? index : collectionKeys[index]; key = (collection === collectionKeys) ? index : collectionKeys[index];
value = collection[key]; value = collection[key];
block = nextBlockOrder[index]; 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 // if we have already seen this object, then we need to reuse the
// associated scope/element // associated scope/element
childScope = block.scope; childScope = block.scope;
@ -342,11 +342,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
nextNode = nextNode.nextSibling; nextNode = nextNode.nextSibling;
} while(nextNode && nextNode[NG_REMOVED]); } while(nextNode && nextNode[NG_REMOVED]);
if (block.startNode != nextNode) { if (getBlockStart(block) != nextNode) {
// existing item which got moved // 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 { } else {
// new item which we don't know about // new item which we don't know about
childScope = $scope.$new(); childScope = $scope.$new();
@ -362,14 +362,16 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
childScope.$odd = !(childScope.$even = (index&1) === 0); childScope.$odd = !(childScope.$even = (index&1) === 0);
// jshint bitwise: true // jshint bitwise: true
if (!block.startNode) { if (!block.scope) {
$transclude(childScope, function(clone) { $transclude(childScope, function(clone) {
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' '); clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
$animate.enter(clone, null, jqLite(previousNode)); $animate.enter(clone, null, jqLite(previousNode));
previousNode = clone; previousNode = clone;
block.scope = childScope; block.scope = childScope;
block.startNode = previousNode && previousNode.endNode ? previousNode.endNode : clone[0]; // Note: We only need the first/last node of the cloned nodes.
block.endNode = clone[clone.length - 1]; // 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; 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];
}
}]; }];

View file

@ -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() { it('should resolve widgets after cloning in inline mode', function() {
module(function($exceptionHandlerProvider) { 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( it('should be implicitly terminal and not compile placeholder content in append', inject(
function($compile, $templateCache, $rootScope, log) { function($compile, $templateCache, $rootScope, log) {

View file

@ -1,8 +1,11 @@
'use strict'; 'use strict';
describe('ngIf', function () { describe('ngIf', function () {
var $scope, $compile, element; var $scope, $compile, element, $compileProvider;
beforeEach(module(function(_$compileProvider_) {
$compileProvider = _$compileProvider_;
}));
beforeEach(inject(function ($rootScope, _$compile_) { beforeEach(inject(function ($rootScope, _$compile_) {
$scope = $rootScope.$new(); $scope = $rootScope.$new();
$compile = _$compile_; $compile = _$compile_;
@ -146,6 +149,28 @@ describe('ngIf', function () {
expect(element.children()[0].className).toContain('my-class'); 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() { describe('ngIf and transcludes', function() {

View file

@ -749,8 +749,30 @@ describe('ngRepeat', function() {
expect(element.text()).toBe('123'); 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) { it('should add separator comments after each item', inject(function ($compile, $rootScope) {
var check = function () { var check = function () {