Skip to content

Commit

Permalink
fix($compile): fix scoping of transclusion directives inside replace …
Browse files Browse the repository at this point in the history
…directive

Closes angular#12975
Closes angular#12936
Closes angular#13244
  • Loading branch information
mzdunek93 authored and gkalpak committed Nov 23, 2015
1 parent 900d351 commit dc677b0
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 7 deletions.
33 changes: 26 additions & 7 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1293,6 +1293,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,
Expand Down Expand Up @@ -1874,7 +1882,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;
}
}
Expand Down Expand Up @@ -1917,8 +1926,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);
Expand Down Expand Up @@ -2213,10 +2225,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});
}
}

Expand Down Expand Up @@ -2363,7 +2380,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);
Expand Down
56 changes: 56 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5636,6 +5636,62 @@ describe('$compile', function() {
});
});

//see issue https://github.com/angular/angular.js/issues/12936
it('should use the proper scope when it is on the root element of a replaced directive template', function() {
module(function() {
directive('isolate', valueFn({
scope: {},
replace: true,
template: '<div trans>{{x}}</div>',
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('<isolate></isolate>')($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 it is on the root element of a replaced directive template with child scope', function() {
module(function() {
directive('child', valueFn({
scope: true,
replace: true,
template: '<div trans>{{x}}</div>',
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('<child></child>')($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() {
Expand Down

0 comments on commit dc677b0

Please sign in to comment.