Skip to content

Commit 25586dc

Browse files
NarretzNarretz
committed
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 actual 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, date, month, time, datetime-local, week, do no longer set $error[inputType], and no longer set the class `ng-invalid-[inputType]`. Instead, they set $error.parse and `ng-invalid-parse`. Closes angular#14292 Closes angular#10076
1 parent 55ba449 commit 25586dc

File tree

4 files changed

+120
-11
lines changed

4 files changed

+120
-11
lines changed

‎src/ng/directive/input.js

+13-9
Original file line numberDiff line numberDiff line change
@@ -1429,12 +1429,11 @@ function createDateParser(regexp, mapping) {
14291429

14301430
function createDateInputType(type, regexp, parseDate, format) {
14311431
return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter) {
1432-
badInputChecker(scope, element, attr, ctrl);
1432+
badInputChecker(scope, element, attr, ctrl, type);
14331433
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
14341434
var timezone = ctrl && ctrl.$options.getOption('timezone');
14351435
var previousDate;
14361436

1437-
ctrl.$$parserName = type;
14381437
ctrl.$parsers.push(function(value) {
14391438
if (ctrl.$isEmpty(value)) return null;
14401439
if (regexp.test(value)) {
@@ -1447,6 +1446,7 @@ function createDateInputType(type, regexp, parseDate, format) {
14471446
}
14481447
return parsedDate;
14491448
}
1449+
ctrl.$$parserName = type;
14501450
return undefined;
14511451
});
14521452

@@ -1499,22 +1499,28 @@ function createDateInputType(type, regexp, parseDate, format) {
14991499
};
15001500
}
15011501

1502-
function badInputChecker(scope, element, attr, ctrl) {
1502+
function badInputChecker(scope, element, attr, ctrl, parserName) {
15031503
var node = element[0];
15041504
var nativeValidation = ctrl.$$hasNativeValidators = isObject(node.validity);
15051505
if (nativeValidation) {
15061506
ctrl.$parsers.push(function(value) {
15071507
var validity = element.prop(VALIDITY_STATE_PROPERTY) || {};
1508-
return validity.badInput || validity.typeMismatch ? undefined : value;
1508+
if (validity.badInput || validity.typeMismatch) {
1509+
ctrl.$$parserName = parserName;
1510+
return undefined;
1511+
}
1512+
1513+
return value;
15091514
});
15101515
}
15111516
}
15121517

15131518
function numberFormatterParser(ctrl) {
1514-
ctrl.$$parserName = 'number';
15151519
ctrl.$parsers.push(function(value) {
15161520
if (ctrl.$isEmpty(value)) return null;
15171521
if (NUMBER_REGEXP.test(value)) return parseFloat(value);
1522+
1523+
ctrl.$$parserName = 'number';
15181524
return undefined;
15191525
});
15201526

@@ -1596,7 +1602,7 @@ function isValidForStep(viewValue, stepBase, step) {
15961602
}
15971603

15981604
function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
1599-
badInputChecker(scope, element, attr, ctrl);
1605+
badInputChecker(scope, element, attr, ctrl, 'number');
16001606
numberFormatterParser(ctrl);
16011607
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
16021608

@@ -1643,7 +1649,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
16431649
}
16441650

16451651
function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
1646-
badInputChecker(scope, element, attr, ctrl);
1652+
badInputChecker(scope, element, attr, ctrl, 'range');
16471653
numberFormatterParser(ctrl);
16481654
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
16491655

@@ -1782,7 +1788,6 @@ function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) {
17821788
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
17831789
stringBasedInputType(ctrl);
17841790

1785-
ctrl.$$parserName = 'url';
17861791
ctrl.$validators.url = function(modelValue, viewValue) {
17871792
var value = modelValue || viewValue;
17881793
return ctrl.$isEmpty(value) || URL_REGEXP.test(value);
@@ -1795,7 +1800,6 @@ function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) {
17951800
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
17961801
stringBasedInputType(ctrl);
17971802

1798-
ctrl.$$parserName = 'email';
17991803
ctrl.$validators.email = function(modelValue, viewValue) {
18001804
var value = modelValue || viewValue;
18011805
return ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value);

‎src/ng/directive/ngModel.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ function NgModelController($scope, $exceptionHandler, $attr, $element, $parse, $
277277
this.$$ngModelSet = this.$$parsedNgModelAssign;
278278
this.$$pendingDebounce = null;
279279
this.$$parserValid = undefined;
280+
this.$$parserName = 'parse';
280281

281282
this.$$currentValidationRunId = 0;
282283

@@ -607,7 +608,8 @@ NgModelController.prototype = {
607608
processAsyncValidators();
608609

609610
function processParseErrors() {
610-
var errorKey = that.$$parserName || 'parse';
611+
var errorKey = that.$$parserName;
612+
611613
if (isUndefined(that.$$parserValid)) {
612614
setValidity(errorKey, null);
613615
} else {
@@ -619,6 +621,7 @@ NgModelController.prototype = {
619621
setValidity(name, null);
620622
});
621623
}
624+
622625
// Set the parse error last, to prevent unsetting it, should a $validators key == parserName
623626
setValidity(errorKey, that.$$parserValid);
624627
return that.$$parserValid;
@@ -721,6 +724,10 @@ NgModelController.prototype = {
721724

722725
this.$$parserValid = isUndefined(modelValue) ? undefined : true;
723726

727+
// Reset any previous parse error
728+
this.$setValidity(this.$$parserName, null);
729+
this.$$parserName = 'parse';
730+
724731
if (this.$$parserValid) {
725732
for (var i = 0; i < this.$parsers.length; i++) {
726733
modelValue = this.$parsers[i](modelValue);

‎test/ng/directive/inputSpec.js

+98
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,37 @@ describe('input', function() {
610610
helper.changeInputValueTo('stuff');
611611
expect(inputElm.val()).toBe('stuff');
612612
expect($rootScope.value).toBeUndefined();
613+
expect(inputElm).toHaveClass('ng-invalid-month');
614+
expect(inputElm).toBeInvalid();
615+
});
616+
617+
618+
it('should not set error=month when a later parser returns undefined', function() {
619+
var inputElm = helper.compileInput('<input type="month" ng-model="value"/>');
620+
var ctrl = inputElm.controller('ngModel');
621+
622+
ctrl.$parsers.push(function() {
623+
return undefined;
624+
});
625+
626+
inputElm[0].setAttribute('type', 'text');
627+
628+
helper.changeInputValueTo('2017-01');
629+
630+
expect($rootScope.value).toBeUndefined();
631+
expect(ctrl.$error.month).toBeFalsy();
632+
expect(ctrl.$error.parse).toBeTruthy();
633+
expect(inputElm).not.toHaveClass('ng-invalid-month');
634+
expect(inputElm).toHaveClass('ng-invalid-parse');
635+
expect(inputElm).toBeInvalid();
636+
637+
helper.changeInputValueTo('asdf');
638+
639+
expect($rootScope.value).toBeUndefined();
640+
expect(ctrl.$error.month).toBeTruthy();
641+
expect(ctrl.$error.parse).toBeFalsy();
642+
expect(inputElm).toHaveClass('ng-invalid-month');
643+
expect(inputElm).not.toHaveClass('ng-invalid-parse');
613644
expect(inputElm).toBeInvalid();
614645
});
615646

@@ -2457,6 +2488,73 @@ describe('input', function() {
24572488
expect($rootScope.value).toBe(123214124123412412e-26);
24582489
});
24592490

2491+
it('should not set $error number if any other parser fails', function() {
2492+
var inputElm = helper.compileInput('<input type="number" ng-model="age"/>');
2493+
var ctrl = inputElm.controller('ngModel');
2494+
2495+
var previousParserFail = false;
2496+
var laterParserFail = false;
2497+
2498+
ctrl.$parsers.unshift(function(value) {
2499+
return previousParserFail ? undefined : value;
2500+
});
2501+
2502+
ctrl.$parsers.push(function(value) {
2503+
return laterParserFail ? undefined : value;
2504+
});
2505+
2506+
// to allow non-number values, we have to change type so that
2507+
// the browser which have number validation will not interfere with
2508+
// this test.
2509+
inputElm[0].setAttribute('type', 'text');
2510+
2511+
helper.changeInputValueTo('123X');
2512+
expect(inputElm.val()).toBe('123X');
2513+
2514+
expect($rootScope.age).toBeUndefined();
2515+
expect(inputElm).toBeInvalid();
2516+
expect(ctrl.$error.number).toBe(true);
2517+
expect(ctrl.$error.parse).toBeFalsy();
2518+
expect(inputElm).toHaveClass('ng-invalid-number');
2519+
expect(inputElm).not.toHaveClass('ng-invalid-parse');
2520+
2521+
previousParserFail = true;
2522+
helper.changeInputValueTo('123');
2523+
expect(inputElm.val()).toBe('123');
2524+
2525+
expect($rootScope.age).toBeUndefined();
2526+
expect(inputElm).toBeInvalid();
2527+
expect(ctrl.$error.number).toBeFalsy();
2528+
expect(ctrl.$error.parse).toBe(true);
2529+
expect(inputElm).not.toHaveClass('ng-invalid-number');
2530+
expect(inputElm).toHaveClass('ng-invalid-parse');
2531+
2532+
previousParserFail = false;
2533+
laterParserFail = true;
2534+
2535+
helper.changeInputValueTo('1234');
2536+
expect(inputElm.val()).toBe('1234');
2537+
2538+
expect($rootScope.age).toBeUndefined();
2539+
expect(inputElm).toBeInvalid();
2540+
expect(ctrl.$error.number).toBeFalsy();
2541+
expect(ctrl.$error.parse).toBe(true);
2542+
expect(inputElm).not.toHaveClass('ng-invalid-number');
2543+
expect(inputElm).toHaveClass('ng-invalid-parse');
2544+
2545+
laterParserFail = false;
2546+
2547+
helper.changeInputValueTo('12345');
2548+
expect(inputElm.val()).toBe('12345');
2549+
2550+
expect($rootScope.age).toBe(12345);
2551+
expect(inputElm).toBeValid();
2552+
expect(ctrl.$error.number).toBeFalsy();
2553+
expect(ctrl.$error.parse).toBeFalsy();
2554+
expect(inputElm).not.toHaveClass('ng-invalid-number');
2555+
expect(inputElm).not.toHaveClass('ng-invalid-parse');
2556+
});
2557+
24602558

24612559
describe('min', function() {
24622560

‎test/ng/directive/ngModelSpec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1378,13 +1378,13 @@ describe('ngModel', function() {
13781378
}
13791379
};
13801380

1381-
ctrl.$$parserName = 'parserOrValidator';
13821381
ctrl.$parsers.push(function(value) {
13831382
switch (value) {
13841383
case 'allInvalid':
13851384
case 'stillAllInvalid':
13861385
case 'parseInvalid-validatorsValid':
13871386
case 'stillParseInvalid-validatorsValid':
1387+
ctrl.$$parserName = 'parserOrValidator';
13881388
return undefined;
13891389
default:
13901390
return value;

0 commit comments

Comments
 (0)