From 908fdaea2370ea101780bd5b1bf44baceab54581 Mon Sep 17 00:00:00 2001 From: mzdunek93 Date: Mon, 28 Sep 2015 21:43:47 +0200 Subject: [PATCH 1/2] fix($compile): fix scoping for transclusion Change the scope's prototype when a directive with transclude is at the root of a template of a replaced directive --- src/ng/compile.js | 34 +++++++++++++++++++++++++++------- test/ng/compileSpec.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 393ab6375a27..1e6ba260e6d9 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1270,7 +1270,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { //================================ - function compile($compileNodes, transcludeFn, maxPriority, ignoreDirective, + function compile($compileNodes, transcludeFn, maxPriority, changeOuterScope, ignoreDirective, previousCompileContext) { if (!($compileNodes instanceof jqLite)) { // jquery always rewraps, whereas we need to preserve the original selector so that we can @@ -1292,6 +1292,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return function publicLinkFn(scope, cloneConnectFn, options) { assertArg(scope, 'scope'); + if (changeOuterScope) { + scope = scope.$parent.$new(false); + } + options = options || {}; var parentBoundTranscludeFn = options.parentBoundTranscludeFn, transcludeControllers = options.transcludeControllers, @@ -1657,16 +1661,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { * @param previousCompileContext * @returns {Function} */ - function compilationGenerator(eager, $compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext) { + function compilationGenerator(eager, $compileNodes, transcludeFn, changeOuterScope, maxPriority, ignoreDirective, previousCompileContext) { if (eager) { - return compile($compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext); + return compile($compileNodes, transcludeFn, maxPriority, changeOuterScope, ignoreDirective, previousCompileContext); } var compiled; return function() { if (!compiled) { - compiled = compile($compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext); + compiled = compile($compileNodes, transcludeFn, maxPriority, changeOuterScope, ignoreDirective, previousCompileContext); // Null out all of these references in order to make them eligible for garbage collection // since this is a potentially long lived closure @@ -1706,6 +1710,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { previousCompileContext = previousCompileContext || {}; var terminalPriority = -Number.MAX_VALUE, + changeOuterScope, newScopeDirective = previousCompileContext.newScopeDirective, controllerDirectives = previousCompileContext.controllerDirectives, newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective, @@ -1796,6 +1801,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (directiveValue = directive.transclude) { hasTranscludeDirective = true; + changeOuterScope = directive.$$replacing; // Special case ngIf and ngRepeat so that we don't complain about duplicate transclusion. // This option should only be used by directives that know how to safely handle element transclusion, @@ -1815,8 +1821,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { compileNode = $compileNode[0]; replaceWith(jqCollection, sliceArgs($template), compileNode); - childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, terminalPriority, - replaceDirective && replaceDirective.name, { + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, changeOuterScope, + terminalPriority, replaceDirective && replaceDirective.name, { // Don't pass in: // - controllerDirectives - otherwise we'll create duplicates controllers // - newIsolateScopeDirective or templateDirective - combining templates with @@ -1829,7 +1835,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } else { $template = jqLite(jqLiteClone(compileNode)).contents(); $compileNode.empty(); // clear contents - childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn); + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, changeOuterScope); } } @@ -1874,6 +1880,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (newIsolateScopeDirective) { markDirectivesAsIsolate(templateDirectives); } + + if (newScopeDirective || newIsolateScopeDirective) { + markDirectivesAsReplacing(templateDirectives); + } + directives = directives.concat(templateDirectives).concat(unprocessedDirectives); mergeTemplateAttributes(templateAttrs, newTemplateAttrs); @@ -2162,6 +2173,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } + function markDirectivesAsReplacing(directives) { + // mark transcluding directives as replacing, so that their outer scope is changed to the directive's scope + for (var j = 0, jj = directives.length; j < jj; j++) { + if (directives[j].transclude) { + directives[j] = inherit(directives[j], {$$replacing: true}); + } + } + } + /** * looks up the directive and decorates it with exception handling and proper parameters. We * call this the boundDirective. diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 54942dfdd1e7..6024e45c856c 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5629,6 +5629,34 @@ 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('parent', 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'); + }); + }); + it('should not leak if two "element" transclusions are on the same element (with debug info)', function() { if (jQuery) { From 2182c879b6c41ac44014ebfb0ee2bb9065a870df Mon Sep 17 00:00:00 2001 From: mzdunek93 Date: Tue, 6 Oct 2015 16:23:43 +0200 Subject: [PATCH 2/2] Transclusion fix update --- src/ng/compile.js | 9 +++++---- test/ng/compileSpec.js | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 1e6ba260e6d9..be68aa2cb576 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1661,7 +1661,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { * @param previousCompileContext * @returns {Function} */ - function compilationGenerator(eager, $compileNodes, transcludeFn, changeOuterScope, maxPriority, ignoreDirective, previousCompileContext) { + function compilationGenerator(eager, $compileNodes, transcludeFn, maxPriority, changeOuterScope, ignoreDirective, previousCompileContext) { if (eager) { return compile($compileNodes, transcludeFn, maxPriority, changeOuterScope, ignoreDirective, previousCompileContext); } @@ -1821,8 +1821,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { compileNode = $compileNode[0]; replaceWith(jqCollection, sliceArgs($template), compileNode); - childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, changeOuterScope, - terminalPriority, replaceDirective && replaceDirective.name, { + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, + terminalPriority, changeOuterScope, replaceDirective && replaceDirective.name, { // Don't pass in: // - controllerDirectives - otherwise we'll create duplicates controllers // - newIsolateScopeDirective or templateDirective - combining templates with @@ -1835,7 +1835,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } else { $template = jqLite(jqLiteClone(compileNode)).contents(); $compileNode.empty(); // clear contents - childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, changeOuterScope); + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, + undefined, changeOuterScope); } } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 6024e45c856c..2246c85d48b8 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5632,7 +5632,7 @@ 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('parent', valueFn({ + directive('isolate', valueFn({ scope: {}, replace: true, template: '
{{x}}
', @@ -5650,7 +5650,7 @@ describe('$compile', function() { })); }); inject(function($rootScope, $compile) { - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.x = 'root'; $rootScope.$apply(); expect(element.text()).toEqual('iso'); @@ -5658,6 +5658,35 @@ 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 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() { if (jQuery) { // jQuery 2.x doesn't expose the cache storage.