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

Commit

Permalink
fix($compile): don't leak isolate scope state when replaced directive…
Browse files Browse the repository at this point in the history
… is used multiple times

When an isolate scope directive is also a "replace" directive and at the root of its template
it has other directives, we need to keep track remember to use isolate scope when linking
these.

This commit fixes the leakage of this state when this directive is used again later inside
or outside of the isolate directive template.
  • Loading branch information
IgorMinar committed Nov 8, 2013
1 parent 3fe4491 commit b5af198
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
13 changes: 11 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1303,13 +1303,17 @@ function $CompileProvider($provide) {
if (pre) {
if (attrStart) pre = groupElementsLinkFnWrapper(pre, attrStart, attrEnd);
pre.require = directive.require;
if (newIsolateScopeDirective === directive || directive.$$isolateScope) pre.isolateScope = true;
if (newIsolateScopeDirective === directive || directive.$$isolateScope) {
pre = cloneAndAnnotateFn(pre, {isolateScope: true});
}
preLinkFns.push(pre);
}
if (post) {
if (attrStart) post = groupElementsLinkFnWrapper(post, attrStart, attrEnd);
post.require = directive.require;
if (newIsolateScopeDirective === directive || directive.$$isolateScope) post.isolateScope = true;
if (newIsolateScopeDirective === directive || directive.$$isolateScope) {
post = cloneAndAnnotateFn(post, {isolateScope: true});
}
postLinkFns.push(post);
}
}
Expand Down Expand Up @@ -1830,6 +1834,11 @@ function $CompileProvider($provide) {
elementsToRemove[0] = newNode;
elementsToRemove.length = 1;
}


function cloneAndAnnotateFn(fn, annotation) {
return extend(function() { return fn.apply(null, arguments); }, fn, annotation);
}
}];
}

Expand Down
39 changes: 37 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2504,7 +2504,7 @@ describe('$compile', function() {
});


it('should share isolate scope with replaced directives', function() {
it('should share isolate scope with replaced directives (template)', function() {
var normalScope;
var isolateScope;

Expand Down Expand Up @@ -2540,7 +2540,7 @@ describe('$compile', function() {
});


it('should share isolate scope with replaced directives', function() {
it('should share isolate scope with replaced directives (templateUrl)', function() {
var normalScope;
var isolateScope;

Expand Down Expand Up @@ -2577,6 +2577,41 @@ describe('$compile', function() {
});


it('should not get confused about where to use isolate scope when a replaced directive is used multiple times',
function() {

module(function() {
directive('isolate', function() {
return {
replace: true,
scope: {},
template: '<span scope-tester="replaced"><span scope-tester="inside"></span></span>'
};
});
directive('scopeTester', function(log) {
return {
link: function($scope, $element) {
log($element.attr('scope-tester') + '=' + ($scope.$root === $scope ? 'non-isolate' : 'isolate'));
}
}
});
});

inject(function($compile, $rootScope, log) {
element = $compile('<div>' +
'<div isolate scope-tester="outside"></div>' +
'<span scope-tester="sibling"></span>' +
'</div>')($rootScope);

$rootScope.$digest();
expect(log).toEqual('inside=isolate; ' +
'outside replaced=non-isolate; ' + // outside
'outside replaced=isolate; ' + // replaced
'sibling=non-isolate')
});
});


it('should require controller of a non-isolate directive from an isolate directive on the ' +
'same element', function() {
var NonIsolateController = function() {};
Expand Down

0 comments on commit b5af198

Please sign in to comment.