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

Commit e7338d3

Browse files
committed
fix($compile): ensure element transclusion directives are linked with comment element
This corrects a complicated compiler issue, described in detail below: Previously, if an element transclusion directive contained an asynchronous directive whose template contained another element transclusion directive, the inner element transclusion directive would be linked with the element, rather than the expected comment node. An example manifestation of this bug would look like so: ```html <div ng-repeat="i in [1,2,3,4,5]"> <div my-directive> </div> </div> ``` `my-directive` would be a replace directive, and its template would contain another element transclusion directive, like so: ```html <div ng-if="true">{{i}}</div> ``` ngIf would be linked with this template content, rather than the comment node, and the template element would be attached to the DOM, rather than the comment. As a result, this caused ng-if to duplicate the template when its expression evaluated to true. Closes #6006 Closes #6101
1 parent 2dfbc08 commit e7338d3

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

src/ng/compile.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -1142,7 +1142,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
11421142
templateDirective = previousCompileContext.templateDirective,
11431143
nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective,
11441144
hasTranscludeDirective = false,
1145-
hasElementTranscludeDirective = false,
1145+
hasElementTranscludeDirective = previousCompileContext.hasElementTranscludeDirective,
11461146
$compileNode = templateAttrs.$$element = jqLite(compileNode),
11471147
directive,
11481148
directiveName,
@@ -1316,6 +1316,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13161316

13171317
nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true;
13181318
nodeLinkFn.transclude = hasTranscludeDirective && childTranscludeFn;
1319+
previousCompileContext.hasElementTranscludeDirective = hasElementTranscludeDirective;
13191320

13201321
// might be normal or delayed nodeLinkFn depending on if templateUrl is present
13211322
return nodeLinkFn;
@@ -1712,8 +1713,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17121713

17131714
if (beforeTemplateLinkNode !== beforeTemplateCompileNode) {
17141715
var oldClasses = beforeTemplateLinkNode.className;
1715-
// it was cloned therefore we have to clone as well.
1716-
linkNode = jqLiteClone(compileNode);
1716+
1717+
if (!(previousCompileContext.hasElementTranscludeDirective &&
1718+
origAsyncDirective.replace)) {
1719+
// it was cloned therefore we have to clone as well.
1720+
linkNode = jqLiteClone(compileNode);
1721+
}
1722+
17171723
replaceWith(linkRootElement, jqLite(beforeTemplateLinkNode), linkNode);
17181724

17191725
// Copy in CSS classes from original node

test/ng/compileSpec.js

+51
Original file line numberDiff line numberDiff line change
@@ -3972,6 +3972,57 @@ describe('$compile', function() {
39723972
});
39733973
});
39743974

3975+
// issue #6006
3976+
it('should link directive with $element as a comment node', function() {
3977+
module(function($provide) {
3978+
directive('innerAgain', function(log) {
3979+
return {
3980+
transclude: 'element',
3981+
link: function(scope, element, attr, controllers, transclude) {
3982+
log('innerAgain:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
3983+
transclude(scope, function(clone) {
3984+
element.parent().append(clone);
3985+
});
3986+
}
3987+
};
3988+
});
3989+
directive('inner', function(log) {
3990+
return {
3991+
replace: true,
3992+
templateUrl: 'inner.html',
3993+
link: function(scope, element) {
3994+
log('inner:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
3995+
}
3996+
};
3997+
});
3998+
directive('outer', function(log) {
3999+
return {
4000+
transclude: 'element',
4001+
link: function(scope, element, attrs, controllers, transclude) {
4002+
log('outer:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
4003+
transclude(scope, function(clone) {
4004+
element.parent().append(clone);
4005+
});
4006+
}
4007+
};
4008+
});
4009+
});
4010+
inject(function(log, $compile, $rootScope, $templateCache) {
4011+
$templateCache.put('inner.html', '<div inner-again><p>Content</p></div>');
4012+
element = $compile('<div><div outer><div inner></div></div></div>')($rootScope);
4013+
$rootScope.$digest();
4014+
var child = element.children();
4015+
4016+
expect(log.toArray()).toEqual([
4017+
"outer:#comment:outer:",
4018+
"innerAgain:#comment:innerAgain:",
4019+
"inner:#comment:innerAgain:"]);
4020+
expect(child.length).toBe(1);
4021+
expect(child.contents().length).toBe(2);
4022+
expect(lowercase(nodeName_(child.contents().eq(0)))).toBe('#comment');
4023+
expect(lowercase(nodeName_(child.contents().eq(1)))).toBe('div');
4024+
});
4025+
});
39754026
});
39764027

39774028
it('should safely create transclude comment node and not break with "-->"',

0 commit comments

Comments
 (0)