From 69730db074e956ebe13cfa09e1c8d7521b7fb375 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sat, 20 Sep 2014 21:41:34 +0300 Subject: [PATCH 1/2] fix(input): fix false initial 'number' validation error in input[number] with observed attributes When no value has been set for an input[number] it's viewValue is undefined. This lead to false parse validation errors, when `ctrl.$validate()` was fired before interacting with the element. A typical situation of this happening is when other directives on the same element `$observe()` attribute values. (Such directives are ngRequired, min, max etc.) More specifically, the parser for native HTML5 validation returns undefined when the input is invalid and returns the value unchanged when the input is valid. Thus, when the value itself is undefined, the NgModelController is tricked into believing there is a native validation error. Closes #9106 --- src/ng/directive/input.js | 6 +- test/ng/directive/inputSpec.js | 137 ++++++++++++++++++++++++++++++--- 2 files changed, 129 insertions(+), 14 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 6b67b8198a3f..f5a9b2e4c220 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1161,9 +1161,6 @@ function badInputChecker(scope, element, attr, ctrl) { } function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { - badInputChecker(scope, element, attr, ctrl); - baseInputType(scope, element, attr, ctrl, $sniffer, $browser); - ctrl.$$parserName = 'number'; ctrl.$parsers.push(function(value) { if (ctrl.$isEmpty(value)) return null; @@ -1171,6 +1168,9 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { return undefined; }); + badInputChecker(scope, element, attr, ctrl); + baseInputType(scope, element, attr, ctrl, $sniffer, $browser); + ctrl.$formatters.push(function(value) { if (!ctrl.$isEmpty(value)) { if (!isNumber(value)) { diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index a50c1640cfc5..64f72697b406 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -1140,10 +1140,8 @@ describe('ngModel', function() { dealoc(element); })); - }); - describe('input', function() { var formElm, inputElm, scope, $compile, $sniffer, $browser, changeInputValueTo, currentSpec; @@ -2209,7 +2207,6 @@ describe('input', function() { compileInput(''); expect(scope.value).toBe('12345'); }); - }); @@ -3387,7 +3384,6 @@ describe('input', function() { expect(inputElm).toBeInvalid(); }); - it('should render as blank if null', function() { compileInput(''); @@ -3408,7 +3404,6 @@ describe('input', function() { expect(inputElm.val()).toBe(''); }); - it('should parse empty string to null', function() { compileInput(''); @@ -3419,7 +3414,6 @@ describe('input', function() { expect(inputElm).toBeValid(); }); - it('should only invalidate the model if suffering from bad input when the data is parsed', function() { compileInput('', { valid: false, @@ -3435,7 +3429,6 @@ describe('input', function() { expect(inputElm).toBeInvalid(); }); - it('should validate number if transition from bad input to empty string', function() { var validity = { valid: false, @@ -3450,6 +3443,18 @@ describe('input', function() { expect(inputElm).toBeValid(); }); + it('should be valid when no value has been set even if it is validated', function() { + // This situation isn't common, but it does arise, when other directives + // on the same element observe an attribute (e.g. ngRequired, min, max etc). + + compileInput(''); + + scope.form.alias.$validate(); + + expect(inputElm).toBeValid(); + expect(scope.form.alias.$error.number).toBeFalsy(); + }); + it('should throw if the model value is not a number', function() { expect(function() { scope.value = 'one'; @@ -3457,7 +3462,6 @@ describe('input', function() { }).toThrowMinErr('ngModel', 'numfmt', "Expected `one` to be a number"); }); - describe('min', function() { it('should validate', function() { @@ -3540,7 +3544,6 @@ describe('input', function() { }); }); - describe('max', function() { it('should validate', function() { @@ -3623,7 +3626,6 @@ describe('input', function() { }); }); - describe('required', function() { it('should be valid even if value is 0', function() { @@ -3655,6 +3657,120 @@ describe('input', function() { }); }); + describe('ngRequired', function() { + + describe('when the ngRequired expression initially evaluates to true', function() { + + it('should be valid even if value is 0', function() { + compileInput(''); + + changeInputValueTo('0'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(0); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + + it('should be valid even if value 0 is set from model', function() { + compileInput(''); + + scope.$apply('value = 0'); + + expect(inputElm).toBeValid(); + expect(inputElm.val()).toBe('0'); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + + it('should register required on non boolean elements', function() { + compileInput('
'); + + scope.$apply("value = ''"); + + expect(inputElm).toBeInvalid(); + expect(scope.form.alias.$error.required).toBeTruthy(); + }); + + it('should be invalid when the user enters an invalid value', function() { + compileInput(''); + + changeInputValueTo('Hello, world !'); + expect(inputElm.val()).toBe(''); + expect(inputElm).toBeInvalid(); + expect(scope.form.alias.$error.required).toBeTruthy(); + }); + + it('should change from invalid to valid when the value is empty and the ngRequired expression changes to false', function() { + compileInput(''); + + scope.$apply('ngRequiredExpr = true'); + + expect(inputElm).toBeInvalid(); + expect(scope.value).toBeUndefined(); + expect(scope.form.alias.$error.required).toBeTruthy(); + + scope.$apply('ngRequiredExpr = false'); + + expect(inputElm).toBeValid(); + expect(scope.value).toBeNull(); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + }); + + describe('when the ngRequired expression initially evaluates to false', function() { + + it('should be valid even if value is empty', function() { + compileInput(''); + + expect(inputElm).toBeValid(); + expect(scope.value).toBeNull(); + expect(scope.form.alias.$error.required).toBeFalsy(); + expect(scope.form.alias.$error.number).toBeFalsy(); + }); + + it('should be valid if value is non-empty', function() { + compileInput(''); + + changeInputValueTo('42'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(42); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + + it('should not have the "required" error flag set even if the value is not a number', function() { + compileInput(''); + + changeInputValueTo('Hello, world !'); + expect(inputElm.val()).toBe(''); + expect(inputElm).toBeValid(); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + + it('should not register required on non boolean elements', function() { + compileInput('
'); + + scope.$apply("value = ''"); + + expect(inputElm).toBeValid(); + expect(scope.form.alias.$error.required).toBeFalsy(); + }); + + it('should change from valid to invalid when the value is empty and the ngRequired expression changes to true', function() { + compileInput(''); + + scope.$apply('ngRequiredExpr = false'); + + expect(inputElm).toBeValid(); + expect(scope.value).toBeNull(); + expect(scope.form.alias.$error.required).toBeFalsy(); + + scope.$apply('ngRequiredExpr = true'); + + expect(inputElm).toBeInvalid(); + expect(scope.value).toBeUndefined(); + expect(scope.form.alias.$error.required).toBeTruthy(); + }); + }); + }); + describe('minlength', function() { it('should invalidate values that are shorter than the given minlength', function() { @@ -3700,7 +3816,6 @@ describe('input', function() { }); }); - describe('maxlength', function() { it('should invalidate values that are longer than the given maxlength', function() { From 02a8436eeec1feb0af2fb114ce15de92d0db1e04 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sat, 20 Sep 2014 22:57:03 +0300 Subject: [PATCH 2/2] fix(input): fix false initial 'number' validation error in input[number] with observed attributes When no value has been set for an input[number] it's viewValue is undefined. This lead to false 'number' validation errors, when `ctrl.$validate()` was fired before interacting with the element. A typical situation of this happening is when other directives on the same element `$observe()` attribute values. (Such directives are ngRequired, min, max etc.) More specifically, the parser for native HTML5 validation returns either undefined (when the input is invalid) or the parsed value (when the input is valid). Thus, when the value itself is undefined, the NgModelController is tricked into believing that there is a native validation error. Closes #9106 --- test/ng/directive/inputSpec.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 64f72697b406..e3f80a1701c7 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -3689,15 +3689,6 @@ describe('input', function() { expect(scope.form.alias.$error.required).toBeTruthy(); }); - it('should be invalid when the user enters an invalid value', function() { - compileInput(''); - - changeInputValueTo('Hello, world !'); - expect(inputElm.val()).toBe(''); - expect(inputElm).toBeInvalid(); - expect(scope.form.alias.$error.required).toBeTruthy(); - }); - it('should change from invalid to valid when the value is empty and the ngRequired expression changes to false', function() { compileInput(''); @@ -3735,15 +3726,6 @@ describe('input', function() { expect(scope.form.alias.$error.required).toBeFalsy(); }); - it('should not have the "required" error flag set even if the value is not a number', function() { - compileInput(''); - - changeInputValueTo('Hello, world !'); - expect(inputElm.val()).toBe(''); - expect(inputElm).toBeValid(); - expect(scope.form.alias.$error.required).toBeFalsy(); - }); - it('should not register required on non boolean elements', function() { compileInput('
');