Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 69f42b7

Browse files
IgorMinarIgorMinar
committed
fix($compile): prevent infinite loop w/ replace+transclude directives
Previously if a template contained a directive that had a template (sync or async) and the directive template was to replace the original element and the directive template contained another directive on the root element of this template and this new directive was an element transclude directive then an infinite recursion would follow because the compiler kept on re-adding and reapplying the original directive to the replaced node. This change fixes that. Closes #2155
1 parent cbbe3bf commit 69f42b7

File tree

2 files changed

+84
-18
lines changed

2 files changed

+84
-18
lines changed

‎src/ng/compile.js

+27-18
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ function $CompileProvider($provide) {
362362

363363
//================================
364364

365-
function compile($compileNodes, transcludeFn, maxPriority) {
365+
function compile($compileNodes, transcludeFn, maxPriority, ignoreDirective) {
366366
if (!($compileNodes instanceof jqLite)) {
367367
// jquery always rewraps, whereas we need to preserve the original selector so that we can modify it.
368368
$compileNodes = jqLite($compileNodes);
@@ -375,7 +375,7 @@ function $CompileProvider($provide) {
375375
$compileNodes[index] = node = jqLite(node).wrap('<span></span>').parent()[0];
376376
}
377377
});
378-
var compositeLinkFn = compileNodes($compileNodes, transcludeFn, $compileNodes, maxPriority);
378+
var compositeLinkFn = compileNodes($compileNodes, transcludeFn, $compileNodes, maxPriority, ignoreDirective);
379379
return function publicLinkFn(scope, cloneConnectFn){
380380
assertArg(scope, 'scope');
381381
// important!!: we must call our jqLite.clone() since the jQuery one is trying to be smart
@@ -422,15 +422,15 @@ function $CompileProvider($provide) {
422422
* @param {number=} max directive priority
423423
* @returns {?function} A composite linking function of all of the matched directives or null.
424424
*/
425-
function compileNodes(nodeList, transcludeFn, $rootElement, maxPriority) {
425+
function compileNodes(nodeList, transcludeFn, $rootElement, maxPriority, ignoreDirective) {
426426
var linkFns = [],
427427
nodeLinkFn, childLinkFn, directives, attrs, linkFnFound;
428428

429429
for(var i = 0; i < nodeList.length; i++) {
430430
attrs = new Attributes();
431431

432432
// we must always refer to nodeList[i] since the nodes can be replaced underneath us.
433-
directives = collectDirectives(nodeList[i], [], attrs, i == 0 ? maxPriority : undefined);
433+
directives = collectDirectives(nodeList[i], [], attrs, i == 0 ? maxPriority : undefined, ignoreDirective);
434434

435435
nodeLinkFn = (directives.length)
436436
? applyDirectivesToNode(directives, nodeList[i], attrs, transcludeFn, $rootElement)
@@ -504,7 +504,7 @@ function $CompileProvider($provide) {
504504
* @param attrs The shared attrs object which is used to populate the normalized attributes.
505505
* @param {number=} maxPriority Max directive priority.
506506
*/
507-
function collectDirectives(node, directives, attrs, maxPriority) {
507+
function collectDirectives(node, directives, attrs, maxPriority, ignoreDirective) {
508508
var nodeType = node.nodeType,
509509
attrsMap = attrs.$attr,
510510
match,
@@ -514,7 +514,7 @@ function $CompileProvider($provide) {
514514
case 1: /* Element */
515515
// use the node name: <directive>
516516
addDirective(directives,
517-
directiveNormalize(nodeName_(node).toLowerCase()), 'E', maxPriority);
517+
directiveNormalize(nodeName_(node).toLowerCase()), 'E', maxPriority, ignoreDirective);
518518

519519
// iterate over the attributes
520520
for (var attr, name, nName, ngAttrName, value, nAttrs = node.attributes,
@@ -545,7 +545,7 @@ function $CompileProvider($provide) {
545545
attrs[nName] = true; // presence means true
546546
}
547547
addAttrInterpolateDirective(node, directives, value, nName);
548-
addDirective(directives, nName, 'A', maxPriority, attrStartName, attrEndName);
548+
addDirective(directives, nName, 'A', maxPriority, ignoreDirective, attrStartName, attrEndName);
549549
}
550550
}
551551

@@ -554,7 +554,7 @@ function $CompileProvider($provide) {
554554
if (isString(className) && className !== '') {
555555
while (match = CLASS_DIRECTIVE_REGEXP.exec(className)) {
556556
nName = directiveNormalize(match[2]);
557-
if (addDirective(directives, nName, 'C', maxPriority)) {
557+
if (addDirective(directives, nName, 'C', maxPriority, ignoreDirective)) {
558558
attrs[nName] = trim(match[3]);
559559
}
560560
className = className.substr(match.index + match[0].length);
@@ -569,7 +569,7 @@ function $CompileProvider($provide) {
569569
match = COMMENT_DIRECTIVE_REGEXP.exec(node.nodeValue);
570570
if (match) {
571571
nName = directiveNormalize(match[1]);
572-
if (addDirective(directives, nName, 'M', maxPriority)) {
572+
if (addDirective(directives, nName, 'M', maxPriority, ignoreDirective)) {
573573
attrs[nName] = trim(match[2]);
574574
}
575575
}
@@ -643,7 +643,7 @@ function $CompileProvider($provide) {
643643
* argument has the root jqLite array so that we can replace nodes on it.
644644
* @returns linkFn
645645
*/
646-
function applyDirectivesToNode(directives, compileNode, templateAttrs, transcludeFn, jqCollection) {
646+
function applyDirectivesToNode(directives, compileNode, templateAttrs, transcludeFn, jqCollection, originalReplaceDirective) {
647647
var terminalPriority = -Number.MAX_VALUE,
648648
preLinkFns = [],
649649
postLinkFns = [],
@@ -655,6 +655,7 @@ function $CompileProvider($provide) {
655655
directiveName,
656656
$template,
657657
transcludeDirective,
658+
replaceDirective = originalReplaceDirective,
658659
childTranscludeFn = transcludeFn,
659660
controllerDirectives,
660661
linkFn,
@@ -705,7 +706,9 @@ function $CompileProvider($provide) {
705706
jqLite(document.createComment(' ' + directiveName + ': ' + templateAttrs[directiveName] + ' '));
706707
compileNode = $compileNode[0];
707708
replaceWith(jqCollection, jqLite(sliceArgs($template)), compileNode);
708-
childTranscludeFn = compile($template, transcludeFn, terminalPriority);
709+
710+
childTranscludeFn = compile($template, transcludeFn, terminalPriority,
711+
replaceDirective && replaceDirective.name);
709712
} else {
710713
$template = jqLite(JQLiteClone(compileNode)).contents();
711714
$compileNode.html(''); // clear contents
@@ -724,6 +727,7 @@ function $CompileProvider($provide) {
724727
directiveValue = denormalizeTemplate(directiveValue);
725728

726729
if (directive.replace) {
730+
replaceDirective = directive;
727731
$template = jqLite('<div>' +
728732
trim(directiveValue) +
729733
'</div>').contents();
@@ -760,9 +764,12 @@ function $CompileProvider($provide) {
760764
if (directive.templateUrl) {
761765
assertNoDuplicate('template', templateDirective, directive, $compileNode);
762766
templateDirective = directive;
767+
768+
if (directive.replace) {
769+
replaceDirective = directive;
770+
}
763771
nodeLinkFn = compileTemplateUrl(directives.splice(i, directives.length - i),
764-
nodeLinkFn, $compileNode, templateAttrs, jqCollection, directive.replace,
765-
childTranscludeFn);
772+
nodeLinkFn, $compileNode, templateAttrs, jqCollection, childTranscludeFn);
766773
ii = directives.length;
767774
} else if (directive.compile) {
768775
try {
@@ -978,7 +985,8 @@ function $CompileProvider($provide) {
978985
* * `M`: comment
979986
* @returns true if directive was added.
980987
*/
981-
function addDirective(tDirectives, name, location, maxPriority, startAttrName, endAttrName) {
988+
function addDirective(tDirectives, name, location, maxPriority, ignoreDirective, startAttrName, endAttrName) {
989+
if (name === ignoreDirective) return null;
982990
var match = null;
983991
if (hasDirectives.hasOwnProperty(name)) {
984992
for(var directive, directives = $injector.get(name + Suffix),
@@ -1039,15 +1047,15 @@ function $CompileProvider($provide) {
10391047

10401048

10411049
function compileTemplateUrl(directives, beforeTemplateNodeLinkFn, $compileNode, tAttrs,
1042-
$rootElement, replace, childTranscludeFn) {
1050+
$rootElement, childTranscludeFn) {
10431051
var linkQueue = [],
10441052
afterTemplateNodeLinkFn,
10451053
afterTemplateChildLinkFn,
10461054
beforeTemplateCompileNode = $compileNode[0],
10471055
origAsyncDirective = directives.shift(),
10481056
// The fact that we have to copy and patch the directive seems wrong!
10491057
derivedSyncDirective = extend({}, origAsyncDirective, {
1050-
controller: null, templateUrl: null, transclude: null, scope: null
1058+
controller: null, templateUrl: null, transclude: null, scope: null, replace: null
10511059
}),
10521060
templateUrl = (isFunction(origAsyncDirective.templateUrl))
10531061
? origAsyncDirective.templateUrl($compileNode, tAttrs)
@@ -1061,7 +1069,7 @@ function $CompileProvider($provide) {
10611069

10621070
content = denormalizeTemplate(content);
10631071

1064-
if (replace) {
1072+
if (origAsyncDirective.replace) {
10651073
$template = jqLite('<div>' + trim(content) + '</div>').contents();
10661074
compileNode = $template[0];
10671075

@@ -1080,7 +1088,8 @@ function $CompileProvider($provide) {
10801088
}
10811089

10821090
directives.unshift(derivedSyncDirective);
1083-
afterTemplateNodeLinkFn = applyDirectivesToNode(directives, compileNode, tAttrs, childTranscludeFn, $compileNode);
1091+
1092+
afterTemplateNodeLinkFn = applyDirectivesToNode(directives, compileNode, tAttrs, childTranscludeFn, $compileNode, origAsyncDirective);
10841093
forEach($rootElement, function(node, i) {
10851094
if (node == compileNode) {
10861095
$rootElement[i] = $compileNode[0];

‎test/ng/directive/ngRepeatSpec.js

+57
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,63 @@ describe('ngRepeat', function() {
454454

455455
describe('nesting in replaced directive templates', function() {
456456

457+
it('should work when placed on a non-root element of attr directive with SYNC replaced template',
458+
inject(function($templateCache, $compile, $rootScope) {
459+
$compileProvider.directive('rr', function() {
460+
return {
461+
restrict: 'A',
462+
replace: true,
463+
template: '<div ng-repeat="i in items">{{i}}|</div>'
464+
};
465+
});
466+
element = jqLite('<div><span rr>{{i}}|</span></div>');
467+
$compile(element)($rootScope);
468+
$rootScope.$apply();
469+
expect(element.text()).toBe('');
470+
471+
$rootScope.items = [1, 2];
472+
$rootScope.$apply();
473+
expect(element.text()).toBe('1|2|');
474+
expect(sortedHtml(element)).toBe(
475+
'<div>' +
476+
'<!-- ngRepeat: i in items -->' +
477+
'<div ng-repeat="i in items" rr="">1|</div>' +
478+
'<div ng-repeat="i in items" rr="">2|</div>' +
479+
'</div>'
480+
);
481+
}));
482+
483+
484+
it('should work when placed on a non-root element of attr directive with ASYNC replaced template',
485+
inject(function($templateCache, $compile, $rootScope) {
486+
$compileProvider.directive('rr', function() {
487+
return {
488+
restrict: 'A',
489+
replace: true,
490+
templateUrl: 'rr.html'
491+
};
492+
});
493+
494+
$templateCache.put('rr.html', '<div ng-repeat="i in items">{{i}}|</div>');
495+
496+
element = jqLite('<div><span rr>{{i}}|</span></div>');
497+
$compile(element)($rootScope);
498+
$rootScope.$apply();
499+
expect(element.text()).toBe('');
500+
501+
$rootScope.items = [1, 2];
502+
$rootScope.$apply();
503+
expect(element.text()).toBe('1|2|');
504+
expect(sortedHtml(element)).toBe(
505+
'<div>' +
506+
'<!-- ngRepeat: i in items -->' +
507+
'<div ng-repeat="i in items" rr="">1|</div>' +
508+
'<div ng-repeat="i in items" rr="">2|</div>' +
509+
'</div>'
510+
);
511+
}));
512+
513+
457514
it('should work when placed on a root element of attr directive with SYNC replaced template',
458515
inject(function($templateCache, $compile, $rootScope) {
459516
$compileProvider.directive('replaceMeWithRepeater', function() {

0 commit comments

Comments
 (0)