Skip to content

Commit 4196ed4

Browse files
mzdunek93petebacondarwin
authored andcommitted
fix($compile): fix scoping of transclusion directives inside replace directive
Closes angular#12975 Closes angular#12936
1 parent aff74ec commit 4196ed4

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
@@ -1292,6 +1292,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
12921292
return function publicLinkFn(scope, cloneConnectFn, options) {
12931293
assertArg(scope, 'scope');
12941294

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

18771885
$compileNode.empty(); // clear contents
1878-
childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn);
1886+
childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, undefined,
1887+
undefined, { needsNewScope: directive.$$isolateScope || directive.$$newScope});
18791888
childTranscludeFn.$$slots = slots;
18801889
}
18811890
}
@@ -1918,8 +1927,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19181927
var templateDirectives = collectDirectives(compileNode, [], newTemplateAttrs);
19191928
var unprocessedDirectives = directives.splice(i + 1, directives.length - (i + 1));
19201929

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

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

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

23662383
if (isObject(origAsyncDirective.scope)) {
2367-
markDirectivesAsIsolate(templateDirectives);
2384+
// the original directive that caused the template to be loaded async required
2385+
// an isolate scope
2386+
markDirectiveScope(templateDirectives, true);
23682387
}
23692388
directives = templateDirectives.concat(directives);
23702389
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 being the root element of a replaced directive', 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 being the root element of a replaced directive 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)