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

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 #13021
Closes #13025
  • Loading branch information
gkalpak authored and petebacondarwin committed Nov 10, 2015
1 parent 7503441 commit b9f7c45
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 @@ -1977,7 +1977,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 @@ -1988,8 +1988,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 @@ -2026,7 +2029,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 @@ -2037,7 +2040,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 b9f7c45

Please sign in to comment.