Skip to content

Commit bd1795e

Browse files
committed
fix(ngModel): fix issues when parserName is same as validator key
For $validate(), it is necessary to store the parseError state in the controller. Otherwise, if the parser name equals a validator key, $validate() will assume a parse error occured if the validator is invalid. Also, setting the validity for the parser now happens after setting validity for the validator key. Otherwise, the parse key is set, and then immediately afterwards the validator key is unset (because parse errors remove all other validations). Fixes angular#10698 Closes angular#10828
1 parent 833ea05 commit bd1795e

File tree

2 files changed

+83
-3
lines changed

2 files changed

+83
-3
lines changed

src/ng/directive/ngModel.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
233233
this.$dirty = false;
234234
this.$valid = true;
235235
this.$invalid = false;
236+
this.$$parseError = false;
236237
this.$error = {}; // keep invalid keys here
237238
this.$$success = {}; // keep valid keys here
238239
this.$pending = undefined; // keep pending keys here
@@ -518,7 +519,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
518519

519520
// Check if the there's a parse error, so we don't unset it accidentially
520521
var parserName = ctrl.$$parserName || 'parse';
521-
var parserValid = ctrl.$error[parserName] ? false : undefined;
522+
var parserValid = ctrl.$$parseError ? false : undefined;
522523

523524
var prevValid = ctrl.$valid;
524525
var prevModelValue = ctrl.$modelValue;
@@ -563,17 +564,21 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
563564
if (parseValid === undefined) {
564565
setValidity(errorKey, null);
565566
} else {
566-
setValidity(errorKey, parseValid);
567567
if (!parseValid) {
568568
forEach(ctrl.$validators, function(v, name) {
569569
setValidity(name, null);
570570
});
571571
forEach(ctrl.$asyncValidators, function(v, name) {
572572
setValidity(name, null);
573573
});
574-
return false;
574+
ctrl.$$parseError = true;
575575
}
576+
// Set the parse error last, to prevent unsetting it, should a $validators key == parserName
577+
setValidity(errorKey, parseValid);
578+
return parseValid;
576579
}
580+
581+
ctrl.$$parseError = false;
577582
return true;
578583
}
579584

@@ -814,6 +819,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
814819
// TODO(perf): why not move this to the action fn?
815820
if (modelValue !== ctrl.$modelValue) {
816821
ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;
822+
ctrl.$$parseError = false;
817823

818824
var formatters = ctrl.$formatters,
819825
idx = formatters.length;

test/ng/directive/ngModelSpec.js

+74
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,80 @@ describe('ngModel', function() {
12211221
expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'ab');
12221222
expect(ctrl.$validators.mock.calls.length).toEqual(2);
12231223
});
1224+
1225+
it('should validate correctly when $parser equals $validator key', function() {
1226+
1227+
var testValid, otherValid, parseValid;
1228+
1229+
ctrl.$validators.test = function(value) {
1230+
return testValid;
1231+
};
1232+
1233+
ctrl.$validators.other = function(value) {
1234+
return otherValid;
1235+
};
1236+
1237+
ctrl.$$parserName = 'test';
1238+
ctrl.$parsers.push(function(value) {
1239+
return parseValid ? true : undefined;
1240+
});
1241+
1242+
1243+
testValid = otherValid = parseValid = false;
1244+
scope.$apply('value = "allInvalid"');
1245+
expect(scope.value).toBe('allInvalid');
1246+
expect(ctrl.$error).toEqual({test: true, other: true});
1247+
1248+
ctrl.$validate();
1249+
expect(scope.value).toBe('allInvalid');
1250+
expect(ctrl.$error).toEqual({test: true, other: true});
1251+
1252+
ctrl.$setViewValue('stillAllInvalid');
1253+
expect(scope.value).toBeUndefined();
1254+
expect(ctrl.$error).toEqual({test: true});
1255+
1256+
ctrl.$validate();
1257+
expect(scope.value).toBeUndefined();
1258+
expect(ctrl.$error).toEqual({test: true});
1259+
1260+
1261+
parseValid = true;
1262+
scope.$apply('value = "parseValidAndValidatorInvalid"');
1263+
expect(scope.value).toBe('parseValidAndValidatorInvalid');
1264+
expect(ctrl.$error).toEqual({test: true, other: true});
1265+
1266+
ctrl.$validate();
1267+
expect(scope.value).toBe('parseValidAndValidatorInvalid');
1268+
expect(ctrl.$error).toEqual({test: true, other: true});
1269+
1270+
ctrl.$setViewValue('stillParseValidAndValidatorInvalid');
1271+
expect(scope.value).toBeUndefined();
1272+
expect(ctrl.$error).toEqual({test: true, other: true});
1273+
1274+
ctrl.$validate();
1275+
expect(scope.value).toBeUndefined();
1276+
expect(ctrl.$error).toEqual({test: true, other: true});
1277+
1278+
1279+
parseValid = false;
1280+
testValid = otherValid = true;
1281+
scope.$apply('value = "parseInvalidAndValidatorValid"');
1282+
expect(scope.value).toBe('parseInvalidAndValidatorValid');
1283+
expect(ctrl.$error).toEqual({});
1284+
1285+
ctrl.$validate();
1286+
expect(scope.value).toBe('parseInvalidAndValidatorValid');
1287+
expect(ctrl.$error).toEqual({});
1288+
1289+
ctrl.$setViewValue('stillParseInvalidAndValidatorValid');
1290+
expect(scope.value).toBeUndefined();
1291+
expect(ctrl.$error).toEqual({test: true});
1292+
1293+
ctrl.$validate();
1294+
expect(scope.value).toBeUndefined();
1295+
expect(ctrl.$error).toEqual({test: true});
1296+
});
1297+
12241298
});
12251299
});
12261300

0 commit comments

Comments
 (0)