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 #10698
Closes #10850
Closes #11046

Author: petebacondarwin

src/ng/directive/ngModel.js

@@ -244,6 +244,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
       ngModelGet = parsedNgModel,
       ngModelSet = parsedNgModelAssign,
       pendingDebounce = null,
+      parserValid,
       ctrl = this;
 
   this.$$setOptions = function(options) {
@@ -516,16 +517,12 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     // the model although neither viewValue nor the model on the scope changed
     var modelValue = ctrl.$$rawModelValue;
 
-    // Check if the there's a parse error, so we don't unset it accidentially
-    var parserName = ctrl.$$parserName || 'parse';
-    var parserValid = ctrl.$error[parserName] ? false : undefined;
-
     var prevValid = ctrl.$valid;
     var prevModelValue = ctrl.$modelValue;
 
     var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
 
-    ctrl.$$runValidators(parserValid, modelValue, viewValue, function(allValid) {
+    ctrl.$$runValidators(modelValue, viewValue, function(allValid) {
       // If there was no change in validity, don't update the model
       // This prevents changing an invalid modelValue to undefined
       if (!allowInvalid && prevValid !== allValid) {
@@ -543,12 +540,12 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
 
   };
 
-  this.$$runValidators = function(parseValid, modelValue, viewValue, doneCallback) {
+  this.$$runValidators = function(modelValue, viewValue, doneCallback) {
     currentValidationRunId++;
     var localValidationRunId = currentValidationRunId;
 
     // check parser error
-    if (!processParseErrors(parseValid)) {
+    if (!processParseErrors()) {
       validationDone(false);
       return;
     }
@@ -558,21 +555,22 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     }
     processAsyncValidators();
 
-    function processParseErrors(parseValid) {
+    function processParseErrors() {
       var errorKey = ctrl.$$parserName || 'parse';
-      if (parseValid === undefined) {
+      if (parserValid === undefined) {
         setValidity(errorKey, null);
       } else {
-        setValidity(errorKey, parseValid);
-        if (!parseValid) {
+        if (!parserValid) {
           forEach(ctrl.$validators, function(v, name) {
             setValidity(name, null);
           });
           forEach(ctrl.$asyncValidators, function(v, name) {
             setValidity(name, null);
           });
-          return false;
         }
+        // Set the parse error last, to prevent unsetting it, should a $validators key == parserName
+        setValidity(errorKey, parserValid);
+        return parserValid;
       }
       return true;
     }
@@ -667,7 +665,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
   this.$$parseAndValidate = function() {
     var viewValue = ctrl.$$lastCommittedViewValue;
     var modelValue = viewValue;
-    var parserValid = isUndefined(modelValue) ? undefined : true;
+    parserValid = isUndefined(modelValue) ? undefined : true;
 
     if (parserValid) {
       for (var i = 0; i < ctrl.$parsers.length; i++) {
@@ -693,7 +691,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
 
     // Pass the $$lastCommittedViewValue here, because the cached viewValue might be out of date.
     // This can happen if e.g. $setViewValue is called from inside a parser
-    ctrl.$$runValidators(parserValid, modelValue, ctrl.$$lastCommittedViewValue, function(allValid) {
+    ctrl.$$runValidators(modelValue, ctrl.$$lastCommittedViewValue, function(allValid) {
       if (!allowInvalid) {
         // Note: Don't check ctrl.$valid here, as we could have
         // external validators (e.g. calculated on the server),
@@ -814,6 +812,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     // TODO(perf): why not move this to the action fn?
     if (modelValue !== ctrl.$modelValue) {
       ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;
+      parserValid = undefined;
 
       var formatters = ctrl.$formatters,
           idx = formatters.length;
@@ -826,7 +825,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
         ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue;
         ctrl.$render();
 
-        ctrl.$$runValidators(undefined, modelValue, viewValue, noop);
+        ctrl.$$runValidators(modelValue, viewValue, noop);
       }
     }
 

test/ng/directive/ngModelSpec.js

@@ -1221,6 +1221,96 @@ describe('ngModel', function() {
         expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'ab');
         expect(ctrl.$validators.mock.calls.length).toEqual(2);
       });
+
+      it('should validate correctly when $parser name equals $validator key', function() {
+
+        ctrl.$validators.parserOrValidator = function(value) {
+          switch (value) {
+            case 'allInvalid':
+            case 'parseValid-validatorsInvalid':
+            case 'stillParseValid-validatorsInvalid':
+              return false;
+            default:
+              return true;
+          }
+        };
+
+        ctrl.$validators.validator = function(value) {
+          switch (value) {
+            case 'allInvalid':
+            case 'parseValid-validatorsInvalid':
+            case 'stillParseValid-validatorsInvalid':
+              return false;
+            default:
+              return true;
+          }
+        };
+
+        ctrl.$$parserName = 'parserOrValidator';
+        ctrl.$parsers.push(function(value) {
+          switch (value) {
+            case 'allInvalid':
+            case 'stillAllInvalid':
+            case 'parseInvalid-validatorsValid':
+            case 'stillParseInvalid-validatorsValid':
+              return undefined;
+            default:
+              return value;
+          }
+        });
+
+        //Parser and validators are invalid
+        scope.$apply('value = "allInvalid"');
+        expect(scope.value).toBe('allInvalid');
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        ctrl.$validate();
+        expect(scope.value).toEqual('allInvalid');
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        ctrl.$setViewValue('stillAllInvalid');
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true});
+
+        ctrl.$validate();
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true});
+
+        //Parser is valid, validators are invalid
+        scope.$apply('value = "parseValid-validatorsInvalid"');
+        expect(scope.value).toBe('parseValid-validatorsInvalid');
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        ctrl.$validate();
+        expect(scope.value).toBe('parseValid-validatorsInvalid');
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        ctrl.$setViewValue('stillParseValid-validatorsInvalid');
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        ctrl.$validate();
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
+
+        //Parser is invalid, validators are valid
+        scope.$apply('value = "parseInvalid-validatorsValid"');
+        expect(scope.value).toBe('parseInvalid-validatorsValid');
+        expect(ctrl.$error).toEqual({});
+
+        ctrl.$validate();
+        expect(scope.value).toBe('parseInvalid-validatorsValid');
+        expect(ctrl.$error).toEqual({});
+
+        ctrl.$setViewValue('stillParseInvalid-validatorsValid');
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true});
+
+        ctrl.$validate();
+        expect(scope.value).toBeUndefined();
+        expect(ctrl.$error).toEqual({parserOrValidator: true});
+      });
+
     });
   });