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

Commit

Permalink
fix($compile): ensure element transclusion directives are linked with…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
caitp committed Feb 10, 2014
1 parent 2dfbc08 commit e7338d3
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1142,7 +1142,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
templateDirective = previousCompileContext.templateDirective,
nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective,
hasTranscludeDirective = false,
hasElementTranscludeDirective = false,
hasElementTranscludeDirective = previousCompileContext.hasElementTranscludeDirective,
$compileNode = templateAttrs.$$element = jqLite(compileNode),
directive,
directiveName,
Expand Down Expand Up @@ -1316,6 +1316,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true;
nodeLinkFn.transclude = hasTranscludeDirective && childTranscludeFn;
previousCompileContext.hasElementTranscludeDirective = hasElementTranscludeDirective;

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

if (beforeTemplateLinkNode !== beforeTemplateCompileNode) {
var oldClasses = beforeTemplateLinkNode.className;
// it was cloned therefore we have to clone as well.
linkNode = jqLiteClone(compileNode);

if (!(previousCompileContext.hasElementTranscludeDirective &&
origAsyncDirective.replace)) {
// it was cloned therefore we have to clone as well.
linkNode = jqLiteClone(compileNode);
}

replaceWith(linkRootElement, jqLite(beforeTemplateLinkNode), linkNode);

// Copy in CSS classes from original node
Expand Down
51 changes: 51 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3972,6 +3972,57 @@ describe('$compile', function() {
});
});

// issue #6006
it('should link directive with $element as a comment node', function() {
module(function($provide) {
directive('innerAgain', function(log) {
return {
transclude: 'element',
link: function(scope, element, attr, controllers, transclude) {
log('innerAgain:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
transclude(scope, function(clone) {
element.parent().append(clone);
});
}
};
});
directive('inner', function(log) {
return {
replace: true,
templateUrl: 'inner.html',
link: function(scope, element) {
log('inner:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
}
};
});
directive('outer', function(log) {
return {
transclude: 'element',
link: function(scope, element, attrs, controllers, transclude) {
log('outer:'+lowercase(nodeName_(element))+':'+trim(element[0].data));
transclude(scope, function(clone) {
element.parent().append(clone);
});
}
};
});
});
inject(function(log, $compile, $rootScope, $templateCache) {
$templateCache.put('inner.html', '<div inner-again><p>Content</p></div>');
element = $compile('<div><div outer><div inner></div></div></div>')($rootScope);
$rootScope.$digest();
var child = element.children();

expect(log.toArray()).toEqual([
"outer:#comment:outer:",
"innerAgain:#comment:innerAgain:",
"inner:#comment:innerAgain:"]);
expect(child.length).toBe(1);
expect(child.contents().length).toBe(2);
expect(lowercase(nodeName_(child.contents().eq(0)))).toBe('#comment');
expect(lowercase(nodeName_(child.contents().eq(1)))).toBe('div');
});
});
});

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

0 comments on commit e7338d3

Please sign in to comment.