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

Commit 1a98c0e

Browse files
mzdunek93petebacondarwin
authored andcommitted
fix($compile): fix scoping of transclusion directives inside replace directive
Closes #12975 Closes #12936 Closes #13244
1 parent b837fc3 commit 1a98c0e

File tree

2 files changed

+82
-7
lines changed

2 files changed

+82
-7
lines changed

src/ng/compile.js

+26-7
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
12931293
return function publicLinkFn(scope, cloneConnectFn, options) {
12941294
assertArg(scope, 'scope');
12951295

1296+
if (previousCompileContext && previousCompileContext.needsNewScope) {
1297+
// A parent directive did a replace and a directive on this element asked
1298+
// for transclusion, which caused us to lose a layer of element on which
1299+
// we could hold the new transclusion scope, so we will create it manually
1300+
// here.
1301+
scope = scope.$parent.$new();
1302+
}
1303+
12961304
options = options || {};
12971305
var parentBoundTranscludeFn = options.parentBoundTranscludeFn,
12981306
transcludeControllers = options.transcludeControllers,
@@ -1874,7 +1882,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
18741882
}
18751883

18761884
$compileNode.empty(); // clear contents
1877-
childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn);
1885+
childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, undefined,
1886+
undefined, { needsNewScope: directive.$$isolateScope || directive.$$newScope});
18781887
childTranscludeFn.$$slots = slots;
18791888
}
18801889
}
@@ -1917,8 +1926,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19171926
var templateDirectives = collectDirectives(compileNode, [], newTemplateAttrs);
19181927
var unprocessedDirectives = directives.splice(i + 1, directives.length - (i + 1));
19191928

1920-
if (newIsolateScopeDirective) {
1921-
markDirectivesAsIsolate(templateDirectives);
1929+
if (newIsolateScopeDirective || newScopeDirective) {
1930+
// The original directive caused the current element to be replaced but this element
1931+
// also needs to have a new scope, so we need to tell the template directives
1932+
// that they would need to get their scope from further up, if they require transclusion
1933+
markDirectiveScope(templateDirectives, newIsolateScopeDirective, newScopeDirective);
19221934
}
19231935
directives = directives.concat(templateDirectives).concat(unprocessedDirectives);
19241936
mergeTemplateAttributes(templateAttrs, newTemplateAttrs);
@@ -2213,10 +2225,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
22132225
}
22142226
}
22152227

2216-
function markDirectivesAsIsolate(directives) {
2217-
// mark all directives as needing isolate scope.
2228+
// Depending upon the context in which a directive finds itself it might need to have a new isolated
2229+
// or child scope created. For instance:
2230+
// * if the directive has been pulled into a template because another directive with a higher priority
2231+
// asked for element transclusion
2232+
// * if the directive itself asks for transclusion but it is at the root of a template and the original
2233+
// element was replaced. See https://github.com/angular/angular.js/issues/12936
2234+
function markDirectiveScope(directives, isolateScope, newScope) {
22182235
for (var j = 0, jj = directives.length; j < jj; j++) {
2219-
directives[j] = inherit(directives[j], {$$isolateScope: true});
2236+
directives[j] = inherit(directives[j], {$$isolateScope: isolateScope, $$newScope: newScope});
22202237
}
22212238
}
22222239

@@ -2363,7 +2380,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
23632380
var templateDirectives = collectDirectives(compileNode, [], tempTemplateAttrs);
23642381

23652382
if (isObject(origAsyncDirective.scope)) {
2366-
markDirectivesAsIsolate(templateDirectives);
2383+
// the original directive that caused the template to be loaded async required
2384+
// an isolate scope
2385+
markDirectiveScope(templateDirectives, true);
23672386
}
23682387
directives = templateDirectives.concat(directives);
23692388
mergeTemplateAttributes(tAttrs, tempTemplateAttrs);

test/ng/compileSpec.js

+56
Original file line numberDiff line numberDiff line change
@@ -5636,6 +5636,62 @@ describe('$compile', function() {
56365636
});
56375637
});
56385638

5639+
//see issue https://github.com/angular/angular.js/issues/12936
5640+
it('should use the proper scope when it is on the root element of a replaced directive template', function() {
5641+
module(function() {
5642+
directive('isolate', valueFn({
5643+
scope: {},
5644+
replace: true,
5645+
template: '<div trans>{{x}}</div>',
5646+
link: function(scope, element, attr, ctrl) {
5647+
scope.x = 'iso';
5648+
}
5649+
}));
5650+
directive('trans', valueFn({
5651+
transclude: 'content',
5652+
link: function(scope, element, attr, ctrl, $transclude) {
5653+
$transclude(function(clone) {
5654+
element.append(clone);
5655+
});
5656+
}
5657+
}));
5658+
});
5659+
inject(function($rootScope, $compile) {
5660+
element = $compile('<isolate></isolate>')($rootScope);
5661+
$rootScope.x = 'root';
5662+
$rootScope.$apply();
5663+
expect(element.text()).toEqual('iso');
5664+
});
5665+
});
5666+
5667+
5668+
//see issue https://github.com/angular/angular.js/issues/12936
5669+
it('should use the proper scope when it is on the root element of a replaced directive template with child scope', function() {
5670+
module(function() {
5671+
directive('child', valueFn({
5672+
scope: true,
5673+
replace: true,
5674+
template: '<div trans>{{x}}</div>',
5675+
link: function(scope, element, attr, ctrl) {
5676+
scope.x = 'child';
5677+
}
5678+
}));
5679+
directive('trans', valueFn({
5680+
transclude: 'content',
5681+
link: function(scope, element, attr, ctrl, $transclude) {
5682+
$transclude(function(clone) {
5683+
element.append(clone);
5684+
});
5685+
}
5686+
}));
5687+
});
5688+
inject(function($rootScope, $compile) {
5689+
element = $compile('<child></child>')($rootScope);
5690+
$rootScope.x = 'root';
5691+
$rootScope.$apply();
5692+
expect(element.text()).toEqual('child');
5693+
});
5694+
});
56395695

56405696

56415697
it('should not leak if two "element" transclusions are on the same element (with debug info)', function() {

0 commit comments

Comments
 (0)