Skip to content

Commit

Permalink
fix($compile): evaluate against the correct scope with bindToControll…
Browse files Browse the repository at this point in the history
…er on new scope

Previously, the directive bindings were evaluated against the directive's
new (non-isolate) scope, instead of the correct (parent) scope.
This went unnoticed most of the time, since a property would be eventually
looked up in the parent scope due to prototypal inheritance. The incorrect
behaviour was exhibited when a property on the child scope was shadowing
that on the parent scope.

This commit fixes it.

Fixes angular#13021
Closes angular#13025
  • Loading branch information
gkalpak authored and petebacondarwin committed Nov 9, 2015
1 parent 11fedaa commit 894cccf
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}

function nodeLinkFn(childLinkFn, scope, linkNode, $rootElement, boundTranscludeFn) {
var linkFn, isolateScope, elementControllers, transcludeFn, $element,
var linkFn, isolateScope, controllerScope, elementControllers, transcludeFn, $element,
attrs, removeScopeBindingWatches, removeControllerBindingWatches;

if (compileNode === linkNode) {
Expand All @@ -2096,8 +2096,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
attrs = new Attributes($element, templateAttrs);
}

controllerScope = scope;
if (newIsolateScopeDirective) {
isolateScope = scope.$new(true);
} else if (newScopeDirective) {
controllerScope = scope.$parent;
}

if (boundTranscludeFn) {
Expand Down Expand Up @@ -2134,7 +2137,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

if (controller.identifier && bindings) {
removeControllerBindingWatches =
initializeDirectiveBindings(scope, attrs, controller.instance, bindings, controllerDirective);
initializeDirectiveBindings(controllerScope, attrs, controller.instance, bindings, controllerDirective);
}

var controllerResult = controller();
Expand All @@ -2145,7 +2148,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
$element.data('$' + controllerDirective.name + 'Controller', controllerResult);
removeControllerBindingWatches && removeControllerBindingWatches();
removeControllerBindingWatches =
initializeDirectiveBindings(scope, attrs, controller.instance, bindings, controllerDirective);
initializeDirectiveBindings(controllerScope, attrs, controller.instance, bindings, controllerDirective);
}
}

Expand Down
118 changes: 118 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4591,6 +4591,124 @@ describe('$compile', function() {
});


it('should evaluate against the correct scope, when using `bindToController` (new scope)',
function() {
module(function($compileProvider, $controllerProvider) {
$controllerProvider.register({
'ParentCtrl': function() {
this.value1 = 'parent1';
this.value2 = 'parent2';
this.value3 = function() { return 'parent3'; };
},
'ChildCtrl': function() {
this.value1 = 'child1';
this.value2 = 'child2';
this.value3 = function() { return 'child3'; };
}
});

$compileProvider.directive('child', valueFn({
scope: true,
controller: 'ChildCtrl as ctrl',
bindToController: {
fromParent1: '@',
fromParent2: '=',
fromParent3: '&'
},
template: ''
}));
});

inject(function($compile, $rootScope) {
element = $compile(
'<div ng-controller="ParentCtrl as ctrl">' +
'<child ' +
'from-parent-1="{{ ctrl.value1 }}" ' +
'from-parent-2="ctrl.value2" ' +
'from-parent-3="ctrl.value3">' +
'</child>' +
'</div>')($rootScope);
$rootScope.$digest();

var parentCtrl = element.controller('ngController');
var childCtrl = element.find('child').controller('child');

expect(childCtrl.fromParent1).toBe(parentCtrl.value1);
expect(childCtrl.fromParent1).not.toBe(childCtrl.value1);
expect(childCtrl.fromParent2).toBe(parentCtrl.value2);
expect(childCtrl.fromParent2).not.toBe(childCtrl.value2);
expect(childCtrl.fromParent3()()).toBe(parentCtrl.value3());
expect(childCtrl.fromParent3()()).not.toBe(childCtrl.value3());

childCtrl.fromParent2 = 'modified';
$rootScope.$digest();

expect(parentCtrl.value2).toBe('modified');
expect(childCtrl.value2).toBe('child2');
});
}
);


it('should evaluate against the correct scope, when using `bindToController` (new iso scope)',
function() {
module(function($compileProvider, $controllerProvider) {
$controllerProvider.register({
'ParentCtrl': function() {
this.value1 = 'parent1';
this.value2 = 'parent2';
this.value3 = function() { return 'parent3'; };
},
'ChildCtrl': function() {
this.value1 = 'child1';
this.value2 = 'child2';
this.value3 = function() { return 'child3'; };
}
});

$compileProvider.directive('child', valueFn({
scope: {},
controller: 'ChildCtrl as ctrl',
bindToController: {
fromParent1: '@',
fromParent2: '=',
fromParent3: '&'
},
template: ''
}));
});

inject(function($compile, $rootScope) {
element = $compile(
'<div ng-controller="ParentCtrl as ctrl">' +
'<child ' +
'from-parent-1="{{ ctrl.value1 }}" ' +
'from-parent-2="ctrl.value2" ' +
'from-parent-3="ctrl.value3">' +
'</child>' +
'</div>')($rootScope);
$rootScope.$digest();

var parentCtrl = element.controller('ngController');
var childCtrl = element.find('child').controller('child');

expect(childCtrl.fromParent1).toBe(parentCtrl.value1);
expect(childCtrl.fromParent1).not.toBe(childCtrl.value1);
expect(childCtrl.fromParent2).toBe(parentCtrl.value2);
expect(childCtrl.fromParent2).not.toBe(childCtrl.value2);
expect(childCtrl.fromParent3()()).toBe(parentCtrl.value3());
expect(childCtrl.fromParent3()()).not.toBe(childCtrl.value3());

childCtrl.fromParent2 = 'modified';
$rootScope.$digest();

expect(parentCtrl.value2).toBe('modified');
expect(childCtrl.value2).toBe('child2');
});
}
);


it('should put controller in scope when controller identifier present but not using controllerAs', function() {
var controllerCalled = false;
var myCtrl;
Expand Down

0 comments on commit 894cccf

Please sign in to comment.