From 4196ed4846d41eee0aee43ff41c3f5e8b881db52 Mon Sep 17 00:00:00 2001 From: mzdunek93 Date: Tue, 3 Nov 2015 22:44:53 +0000 Subject: [PATCH] fix($compile): fix scoping of transclusion directives inside replace directive Closes #12975 Closes #12936 --- src/ng/compile.js | 33 +++++++++++++++++++------ test/ng/compileSpec.js | 56 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 8b6491091de7..52592d3792b3 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1292,6 +1292,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return function publicLinkFn(scope, cloneConnectFn, options) { assertArg(scope, 'scope'); + if (previousCompileContext && previousCompileContext.needsNewScope) { + // A parent directive did a replace and a directive on this element asked + // for transclusion, which caused us to lose a layer of element on which + // we could hold the new transclusion scope, so we will create it manually + // here. + scope = scope.$parent.$new(); + } + options = options || {}; var parentBoundTranscludeFn = options.parentBoundTranscludeFn, transcludeControllers = options.transcludeControllers, @@ -1875,7 +1883,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } $compileNode.empty(); // clear contents - childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn); + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, undefined, + undefined, { needsNewScope: directive.$$isolateScope || directive.$$newScope}); childTranscludeFn.$$slots = slots; } } @@ -1918,8 +1927,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var templateDirectives = collectDirectives(compileNode, [], newTemplateAttrs); var unprocessedDirectives = directives.splice(i + 1, directives.length - (i + 1)); - if (newIsolateScopeDirective) { - markDirectivesAsIsolate(templateDirectives); + if (newIsolateScopeDirective || newScopeDirective) { + // The original directive caused the current element to be replaced but this element + // also needs to have a new scope, so we need to tell the template directives + // that they would need to get their scope from further up, if they require transclusion + markDirectiveScope(templateDirectives, newIsolateScopeDirective, newScopeDirective); } directives = directives.concat(templateDirectives).concat(unprocessedDirectives); mergeTemplateAttributes(templateAttrs, newTemplateAttrs); @@ -2214,10 +2226,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } - function markDirectivesAsIsolate(directives) { - // mark all directives as needing isolate scope. + // Depending upon the context in which a directive finds itself it might need to have a new isolated + // or child scope created. For instance: + // * if the directive has been pulled into a template because another directive with a higher priority + // asked for element transclusion + // * if the directive itself asks for transclusion but it is at the root of a template and the original + // element was replaced. See https://github.com/angular/angular.js/issues/12936 + function markDirectiveScope(directives, isolateScope, newScope) { for (var j = 0, jj = directives.length; j < jj; j++) { - directives[j] = inherit(directives[j], {$$isolateScope: true}); + directives[j] = inherit(directives[j], {$$isolateScope: isolateScope, $$newScope: newScope}); } } @@ -2364,7 +2381,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var templateDirectives = collectDirectives(compileNode, [], tempTemplateAttrs); if (isObject(origAsyncDirective.scope)) { - markDirectivesAsIsolate(templateDirectives); + // the original directive that caused the template to be loaded async required + // an isolate scope + markDirectiveScope(templateDirectives, true); } directives = templateDirectives.concat(directives); mergeTemplateAttributes(tAttrs, tempTemplateAttrs); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index da016f74c5ac..a89fa6dd222c 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5636,6 +5636,62 @@ describe('$compile', function() { }); }); + //see issue https://github.com/angular/angular.js/issues/12936 + it('should use the proper scope when being the root element of a replaced directive', function() { + module(function() { + directive('isolate', valueFn({ + scope: {}, + replace: true, + template: '
{{x}}
', + link: function(scope, element, attr, ctrl) { + scope.x = 'iso'; + } + })); + directive('trans', valueFn({ + transclude: 'content', + link: function(scope, element, attr, ctrl, $transclude) { + $transclude(function(clone) { + element.append(clone); + }); + } + })); + }); + inject(function($rootScope, $compile) { + element = $compile('')($rootScope); + $rootScope.x = 'root'; + $rootScope.$apply(); + expect(element.text()).toEqual('iso'); + }); + }); + + + //see issue https://github.com/angular/angular.js/issues/12936 + it('should use the proper scope when being the root element of a replaced directive with child scope', function() { + module(function() { + directive('child', valueFn({ + scope: true, + replace: true, + template: '
{{x}}
', + link: function(scope, element, attr, ctrl) { + scope.x = 'child'; + } + })); + directive('trans', valueFn({ + transclude: 'content', + link: function(scope, element, attr, ctrl, $transclude) { + $transclude(function(clone) { + element.append(clone); + }); + } + })); + }); + inject(function($rootScope, $compile) { + element = $compile('')($rootScope); + $rootScope.x = 'root'; + $rootScope.$apply(); + expect(element.text()).toEqual('child'); + }); + }); it('should not leak if two "element" transclusions are on the same element (with debug info)', function() {