From 08f6f99d2cb67c351b48391873854dcbd85c71ca Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 21 Nov 2017 18:30:41 +0100 Subject: [PATCH] fix(ngModel, input): improve handling of built-in named parsers This commit changes how input elements use the private $$parserName property on the ngModelController to name parse errors. Until now, the input types (number, date etc.) would set $$parserName when the inputs were initialized, which meant that any other parsers on the ngModelController would also be named after that type. The effect of that was that the `$error` property and the `ng-invalid-...` class would always be that of the built-in parser, even if the custom parser had nothing to do with it. The new behavior is that the $$parserName is only set if the built-in parser is invalid i.e. returns `undefined`. Also, $$parserName has been removed from input[email] and input[url], as these types do not have a built-in parser anymore. BREAKING CHANGE: *Custom* parsers that fail to parse on input types "email", "url", "number", "date", "month", "time", "datetime-local", "week", do no longer set `ngModelController.$error[inputType]`, and the `ng-invalid-[inputType]` class. Also, custom parsers on input type "range" do no longer set `ngModelController.$error.number` and the `ng-invalid-number` class. Instead, any custom parsers on these inputs set `ngModelController.$error.parse` and `ng-invalid-parse`. Closes #14292 Closes #10076 --- src/ng/directive/input.js | 22 ++++--- src/ng/directive/ngModel.js | 9 ++- test/ng/directive/inputSpec.js | 98 ++++++++++++++++++++++++++++++++ test/ng/directive/ngModelSpec.js | 2 +- 4 files changed, 120 insertions(+), 11 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index ebae9b3bed18..228f5fb2366a 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1429,12 +1429,11 @@ function createDateParser(regexp, mapping) { function createDateInputType(type, regexp, parseDate, format) { return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter) { - badInputChecker(scope, element, attr, ctrl); + badInputChecker(scope, element, attr, ctrl, type); baseInputType(scope, element, attr, ctrl, $sniffer, $browser); var timezone = ctrl && ctrl.$options.getOption('timezone'); var previousDate; - ctrl.$$parserName = type; ctrl.$parsers.push(function(value) { if (ctrl.$isEmpty(value)) return null; if (regexp.test(value)) { @@ -1447,6 +1446,7 @@ function createDateInputType(type, regexp, parseDate, format) { } return parsedDate; } + ctrl.$$parserName = type; return undefined; }); @@ -1499,22 +1499,28 @@ function createDateInputType(type, regexp, parseDate, format) { }; } -function badInputChecker(scope, element, attr, ctrl) { +function badInputChecker(scope, element, attr, ctrl, parserName) { var node = element[0]; var nativeValidation = ctrl.$$hasNativeValidators = isObject(node.validity); if (nativeValidation) { ctrl.$parsers.push(function(value) { var validity = element.prop(VALIDITY_STATE_PROPERTY) || {}; - return validity.badInput || validity.typeMismatch ? undefined : value; + if (validity.badInput || validity.typeMismatch) { + ctrl.$$parserName = parserName; + return undefined; + } + + return value; }); } } function numberFormatterParser(ctrl) { - ctrl.$$parserName = 'number'; ctrl.$parsers.push(function(value) { if (ctrl.$isEmpty(value)) return null; if (NUMBER_REGEXP.test(value)) return parseFloat(value); + + ctrl.$$parserName = 'number'; return undefined; }); @@ -1596,7 +1602,7 @@ function isValidForStep(viewValue, stepBase, step) { } function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { - badInputChecker(scope, element, attr, ctrl); + badInputChecker(scope, element, attr, ctrl, 'number'); numberFormatterParser(ctrl); baseInputType(scope, element, attr, ctrl, $sniffer, $browser); @@ -1643,7 +1649,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { } function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { - badInputChecker(scope, element, attr, ctrl); + badInputChecker(scope, element, attr, ctrl, 'range'); numberFormatterParser(ctrl); baseInputType(scope, element, attr, ctrl, $sniffer, $browser); @@ -1782,7 +1788,6 @@ function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) { baseInputType(scope, element, attr, ctrl, $sniffer, $browser); stringBasedInputType(ctrl); - ctrl.$$parserName = 'url'; ctrl.$validators.url = function(modelValue, viewValue) { var value = modelValue || viewValue; return ctrl.$isEmpty(value) || URL_REGEXP.test(value); @@ -1795,7 +1800,6 @@ function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) { baseInputType(scope, element, attr, ctrl, $sniffer, $browser); stringBasedInputType(ctrl); - ctrl.$$parserName = 'email'; ctrl.$validators.email = function(modelValue, viewValue) { var value = modelValue || viewValue; return ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value); diff --git a/src/ng/directive/ngModel.js b/src/ng/directive/ngModel.js index 8afa3da7f64a..1dec8d31ed33 100644 --- a/src/ng/directive/ngModel.js +++ b/src/ng/directive/ngModel.js @@ -277,6 +277,7 @@ function NgModelController($scope, $exceptionHandler, $attr, $element, $parse, $ this.$$ngModelSet = this.$$parsedNgModelAssign; this.$$pendingDebounce = null; this.$$parserValid = undefined; + this.$$parserName = 'parse'; this.$$currentValidationRunId = 0; @@ -607,7 +608,8 @@ NgModelController.prototype = { processAsyncValidators(); function processParseErrors() { - var errorKey = that.$$parserName || 'parse'; + var errorKey = that.$$parserName; + if (isUndefined(that.$$parserValid)) { setValidity(errorKey, null); } else { @@ -619,6 +621,7 @@ NgModelController.prototype = { setValidity(name, null); }); } + // Set the parse error last, to prevent unsetting it, should a $validators key == parserName setValidity(errorKey, that.$$parserValid); return that.$$parserValid; @@ -721,6 +724,10 @@ NgModelController.prototype = { this.$$parserValid = isUndefined(modelValue) ? undefined : true; + // Reset any previous parse error + this.$setValidity(this.$$parserName, null); + this.$$parserName = 'parse'; + if (this.$$parserValid) { for (var i = 0; i < this.$parsers.length; i++) { modelValue = this.$parsers[i](modelValue); diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index d1a552194c4e..afc869074a78 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -610,6 +610,37 @@ describe('input', function() { helper.changeInputValueTo('stuff'); expect(inputElm.val()).toBe('stuff'); expect($rootScope.value).toBeUndefined(); + expect(inputElm).toHaveClass('ng-invalid-month'); + expect(inputElm).toBeInvalid(); + }); + + + it('should not set error=month when a later parser returns undefined', function() { + var inputElm = helper.compileInput(''); + var ctrl = inputElm.controller('ngModel'); + + ctrl.$parsers.push(function() { + return undefined; + }); + + inputElm[0].setAttribute('type', 'text'); + + helper.changeInputValueTo('2017-01'); + + expect($rootScope.value).toBeUndefined(); + expect(ctrl.$error.month).toBeFalsy(); + expect(ctrl.$error.parse).toBeTruthy(); + expect(inputElm).not.toHaveClass('ng-invalid-month'); + expect(inputElm).toHaveClass('ng-invalid-parse'); + expect(inputElm).toBeInvalid(); + + helper.changeInputValueTo('asdf'); + + expect($rootScope.value).toBeUndefined(); + expect(ctrl.$error.month).toBeTruthy(); + expect(ctrl.$error.parse).toBeFalsy(); + expect(inputElm).toHaveClass('ng-invalid-month'); + expect(inputElm).not.toHaveClass('ng-invalid-parse'); expect(inputElm).toBeInvalid(); }); @@ -2457,6 +2488,73 @@ describe('input', function() { expect($rootScope.value).toBe(123214124123412412e-26); }); + it('should not set $error number if any other parser fails', function() { + var inputElm = helper.compileInput(''); + var ctrl = inputElm.controller('ngModel'); + + var previousParserFail = false; + var laterParserFail = false; + + ctrl.$parsers.unshift(function(value) { + return previousParserFail ? undefined : value; + }); + + ctrl.$parsers.push(function(value) { + return laterParserFail ? undefined : value; + }); + + // to allow non-number values, we have to change type so that + // the browser which have number validation will not interfere with + // this test. + inputElm[0].setAttribute('type', 'text'); + + helper.changeInputValueTo('123X'); + expect(inputElm.val()).toBe('123X'); + + expect($rootScope.age).toBeUndefined(); + expect(inputElm).toBeInvalid(); + expect(ctrl.$error.number).toBe(true); + expect(ctrl.$error.parse).toBeFalsy(); + expect(inputElm).toHaveClass('ng-invalid-number'); + expect(inputElm).not.toHaveClass('ng-invalid-parse'); + + previousParserFail = true; + helper.changeInputValueTo('123'); + expect(inputElm.val()).toBe('123'); + + expect($rootScope.age).toBeUndefined(); + expect(inputElm).toBeInvalid(); + expect(ctrl.$error.number).toBeFalsy(); + expect(ctrl.$error.parse).toBe(true); + expect(inputElm).not.toHaveClass('ng-invalid-number'); + expect(inputElm).toHaveClass('ng-invalid-parse'); + + previousParserFail = false; + laterParserFail = true; + + helper.changeInputValueTo('1234'); + expect(inputElm.val()).toBe('1234'); + + expect($rootScope.age).toBeUndefined(); + expect(inputElm).toBeInvalid(); + expect(ctrl.$error.number).toBeFalsy(); + expect(ctrl.$error.parse).toBe(true); + expect(inputElm).not.toHaveClass('ng-invalid-number'); + expect(inputElm).toHaveClass('ng-invalid-parse'); + + laterParserFail = false; + + helper.changeInputValueTo('12345'); + expect(inputElm.val()).toBe('12345'); + + expect($rootScope.age).toBe(12345); + expect(inputElm).toBeValid(); + expect(ctrl.$error.number).toBeFalsy(); + expect(ctrl.$error.parse).toBeFalsy(); + expect(inputElm).not.toHaveClass('ng-invalid-number'); + expect(inputElm).not.toHaveClass('ng-invalid-parse'); + }); + describe('min', function() { diff --git a/test/ng/directive/ngModelSpec.js b/test/ng/directive/ngModelSpec.js index 7e839e962865..bc14959ce890 100644 --- a/test/ng/directive/ngModelSpec.js +++ b/test/ng/directive/ngModelSpec.js @@ -1378,13 +1378,13 @@ describe('ngModel', function() { } }; - ctrl.$$parserName = 'parserOrValidator'; ctrl.$parsers.push(function(value) { switch (value) { case 'allInvalid': case 'stillAllInvalid': case 'parseInvalid-validatorsValid': case 'stillParseInvalid-validatorsValid': + ctrl.$$parserName = 'parserOrValidator'; return undefined; default: return value;