diff --git a/src/ng/compile.js b/src/ng/compile.js index 7a926f309fdc..b0b2d63940c8 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2567,24 +2567,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { lastValue, parentGet, parentSet, compare; - if (!hasOwnProperty.call(attrs, attrName)) { - // In the case of user defined a binding with the same name as a method in Object.prototype but didn't set - // the corresponding attribute. We need to make sure subsequent code won't access to the prototype function - attrs[attrName] = undefined; - } - switch (mode) { case '@': - if (!attrs[attrName] && !optional) { - destination[scopeName] = undefined; + if (!optional && !hasOwnProperty.call(attrs, attrName)) { + destination[scopeName] = attrs[attrName] = void 0; } - attrs.$observe(attrName, function(value) { - destination[scopeName] = value; + if (isString(value)) { + destination[scopeName] = value; + } }); attrs.$$observers[attrName].$$scope = scope; - if (attrs[attrName]) { + if (isString(attrs[attrName])) { // If the attribute has been provided then we trigger an interpolation to ensure // the value is there for use in the link fn destination[scopeName] = $interpolate(attrs[attrName])(scope); @@ -2592,11 +2587,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { break; case '=': - if (optional && !attrs[attrName]) { - return; + if (!hasOwnProperty.call(attrs, attrName)) { + if (optional) break; + attrs[attrName] = void 0; } - parentGet = $parse(attrs[attrName]); + if (optional && !attrs[attrName]) break; + parentGet = $parse(attrs[attrName]); if (parentGet.literal) { compare = equals; } else { @@ -2635,7 +2632,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { break; case '&': - parentGet = $parse(attrs[attrName]); + // Don't assign Object.prototype method to scope + parentGet = attrs.hasOwnProperty(attrName) ? $parse(attrs[attrName]) : noop; // Don't assign noop to destination if expression is not valid if (parentGet === noop && optional) break; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 0239b8cd2cac..02ea29cc9545 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -2543,10 +2543,17 @@ describe('$compile', function() { }; expect(func).not.toThrow(); - expect(element.find('span').scope()).toBe(element.isolateScope()); - expect(element.isolateScope()).not.toBe($rootScope); - expect(element.isolateScope()['constructor']).toBe($rootScope.constructor); - expect(element.isolateScope()['valueOf']).toBeUndefined(); + var scope = element.isolateScope(); + expect(element.find('span').scope()).toBe(scope); + expect(scope).not.toBe($rootScope); + + // Not shadowed because optional + expect(scope.constructor).toBe($rootScope.constructor); + expect(scope.hasOwnProperty('constructor')).toBe(false); + + // Shadowed with undefined because not optional + expect(scope.valueOf).toBeUndefined(); + expect(scope.hasOwnProperty('valueOf')).toBe(true); }) ); @@ -2561,10 +2568,13 @@ describe('$compile', function() { }; expect(func).not.toThrow(); - expect(element.find('span').scope()).toBe(element.isolateScope()); - expect(element.isolateScope()).not.toBe($rootScope); - expect(element.isolateScope()['constructor']).toBe('constructor'); - expect(element.isolateScope()['valueOf']).toBe('valueOf'); + var scope = element.isolateScope(); + expect(element.find('span').scope()).toBe(scope); + expect(scope).not.toBe($rootScope); + expect(scope.constructor).toBe('constructor'); + expect(scope.hasOwnProperty('constructor')).toBe(true); + expect(scope.valueOf).toBe('valueOf'); + expect(scope.hasOwnProperty('valueOf')).toBe(true); }) ); @@ -2575,10 +2585,17 @@ describe('$compile', function() { }; expect(func).not.toThrow(); - expect(element.find('span').scope()).toBe(element.isolateScope()); - expect(element.isolateScope()).not.toBe($rootScope); - expect(element.isolateScope()['constructor']).toBe($rootScope.constructor); - expect(element.isolateScope()['valueOf']).toBeUndefined(); + var scope = element.isolateScope(); + expect(element.find('span').scope()).toBe(scope); + expect(scope).not.toBe($rootScope); + + // Does not shadow value because optional + expect(scope.constructor).toBe($rootScope.constructor); + expect(scope.hasOwnProperty('constructor')).toBe(false); + + // Shadows value because not optional + expect(scope.valueOf).toBeUndefined(); + expect(scope.hasOwnProperty('valueOf')).toBe(true); }) ); @@ -3576,6 +3593,58 @@ describe('$compile', function() { })); + it('should not overwrite @-bound property each digest when not present', function() { + module(function($compileProvider) { + $compileProvider.directive('testDir', valueFn({ + scope: {prop: '@'}, + controller: function($scope) { + $scope.prop = $scope.prop || 'default'; + this.getProp = function() { + return $scope.prop; + }; + }, + controllerAs: 'ctrl', + template: '

' + })); + }); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + var scope = element.isolateScope(); + expect(scope.ctrl.getProp()).toBe('default'); + + $rootScope.$digest(); + expect(scope.ctrl.getProp()).toBe('default'); + }); + }); + + + it('should ignore optional "="-bound property if value is the emptry string', function() { + module(function($compileProvider) { + $compileProvider.directive('testDir', valueFn({ + scope: {prop: '=?'}, + controller: function($scope) { + $scope.prop = $scope.prop || 'default'; + this.getProp = function() { + return $scope.prop; + }; + }, + controllerAs: 'ctrl', + template: '

' + })); + }); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + var scope = element.isolateScope(); + expect(scope.ctrl.getProp()).toBe('default'); + $rootScope.$digest(); + expect(scope.ctrl.getProp()).toBe('default'); + scope.prop = 'foop'; + $rootScope.$digest(); + expect(scope.ctrl.getProp()).toBe('foop'); + }); + }); + + describe('bind-once', function() { function countWatches(scope) { @@ -4437,6 +4506,64 @@ describe('$compile', function() { childScope.theCtrl.test(); }); }); + + describe('should not overwrite @-bound property each digest when not present', function() { + it('when creating new scope', function() { + module(function($compileProvider) { + $compileProvider.directive('testDir', valueFn({ + scope: true, + bindToController: { + prop: '@' + }, + controller: function() { + var self = this; + this.prop = this.prop || 'default'; + this.getProp = function() { + return self.prop; + }; + }, + controllerAs: 'ctrl', + template: '

' + })); + }); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + var scope = element.scope(); + expect(scope.ctrl.getProp()).toBe('default'); + + $rootScope.$digest(); + expect(scope.ctrl.getProp()).toBe('default'); + }); + }); + + it('when creating isolate scope', function() { + module(function($compileProvider) { + $compileProvider.directive('testDir', valueFn({ + scope: {}, + bindToController: { + prop: '@' + }, + controller: function() { + var self = this; + this.prop = this.prop || 'default'; + this.getProp = function() { + return self.prop; + }; + }, + controllerAs: 'ctrl', + template: '

' + })); + }); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + var scope = element.isolateScope(); + expect(scope.ctrl.getProp()).toBe('default'); + + $rootScope.$digest(); + expect(scope.ctrl.getProp()).toBe('default'); + }); + }); + }); });