From e7338d3f27e8824196136a18e1c3e0fcf51a0e28 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Tue, 28 Jan 2014 11:34:01 -0500 Subject: [PATCH] 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
``` `my-directive` would be a replace directive, and its template would contain another element transclusion directive, like so: ```html
{{i}}
``` 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 --- src/ng/compile.js | 12 +++++++--- test/ng/compileSpec.js | 51 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 63523c04dac6..de65c83e2c2c 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -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, @@ -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; @@ -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 diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 085eb6e9d552..60b1024bf97f 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -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', '

Content

'); + element = $compile('
')($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 "-->"',