From 50557a6cd329e8438fb5694d11e8a7d018142afe Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Tue, 6 Oct 2015 22:43:14 +0300 Subject: [PATCH] fix($compile): evaluate against the correct scope with bindToController 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 --- src/ng/compile.js | 9 ++-- test/ng/compileSpec.js | 118 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 2376257b5331..b333b079aa82 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2084,7 +2084,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) { @@ -2095,8 +2095,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) { @@ -2133,7 +2136,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(); @@ -2144,7 +2147,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); } } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 136bec90a25b..db48aa665770 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -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( + '
' + + '' + + '' + + '
')($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( + '
' + + '' + + '' + + '
')($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;