fix(ngModel): don't run parsers when executing $validate

src/ng/directive/input.js

@@ -1721,6 +1721,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     function($scope, $exceptionHandler, $attr, $element, $parse, $animate, $timeout, $rootScope, $q, $interpolate) {
   this.$viewValue = Number.NaN;
   this.$modelValue = Number.NaN;
+  this.$rawModelValue = undefined; // stores the parsed modelValue / model set from scope regardless of validity.
   this.$validators = {};
   this.$asyncValidators = {};
   this.$parsers = [];
@@ -1975,14 +1976,51 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
    * @name ngModel.NgModelController#$validate
    *
    * @description
-   * Runs each of the registered validators (first synchronous validators and then asynchronous validators).
+   * Runs each of the registered validators (first synchronous validators and then
+   * asynchronous validators).
+   * If the validity changes to invalid, the model will be set to `undefined`,
+   * unless {@link ngModelOptions `ngModelOptions.allowInvalid`} is `true`.
+   * If the validity changes to valid, it will set the model to the last available valid
+   * modelValue, i.e. either the last parsed value or the last value set from the scope.
    */
   this.$validate = function() {
     // ignore $validate before model is initialized
     if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
       return;
     }
-    this.$$parseAndValidate();
+
+    var viewValue = ctrl.$$lastCommittedViewValue;
+    // Note: we use the $$rawModelValue as $modelValue might have been
+    // set to undefined during a view -> model update that found validation
+    // errors. We can't parse the view here, since that could change
+    // 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) {
+      // If there was no change in validity, don't update the model
+      // This prevents changing an invalid modelValue to undefined
+      if (!allowInvalid && prevValid !== allValid) {
+        // Note: Don't check ctrl.$valid here, as we could have
+        // external validators (e.g. calculated on the server),
+        // that just call $setValidity and need the model value
+        // to calculate their validity.
+        ctrl.$modelValue = allValid ? modelValue : undefined;
+
+        if (ctrl.$modelValue !== prevModelValue) {
+          ctrl.$$writeModelToScope();
+        }
+      }
+    });
+
   };
 
   this.$$runValidators = function(parseValid, modelValue, viewValue, doneCallback) {
@@ -2130,6 +2168,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     }
     var prevModelValue = ctrl.$modelValue;
     var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
+    ctrl.$$rawModelValue = modelValue;
     if (allowInvalid) {
       ctrl.$modelValue = modelValue;
       writeToModelIfNeeded();
@@ -2254,7 +2293,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
     // if scope model value and ngModel value are out of sync
     // TODO(perf): why not move this to the action fn?
     if (modelValue !== ctrl.$modelValue) {
-      ctrl.$modelValue = modelValue;
+      ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;
 
       var formatters = ctrl.$formatters,
           idx = formatters.length;

test/ng/directive/inputSpec.js

@@ -484,72 +484,190 @@ describe('NgModelController', function() {
 
   });
 
-  describe('validations pipeline', function() {
+  describe('validation', function() {
 
-    it('should perform validations when $validate() is called', function() {
-      scope.$apply('value = ""');
+    describe('$validate', function() {
+      it('should perform validations when $validate() is called', function() {
+        scope.$apply('value = ""');
 
-      var validatorResult = false;
-      ctrl.$validators.someValidator = function(value) {
-        return validatorResult;
-      };
+        var validatorResult = false;
+        ctrl.$validators.someValidator = function(value) {
+          return validatorResult;
+        };
 
-      ctrl.$validate();
+        ctrl.$validate();
 
-      expect(ctrl.$valid).toBe(false);
+        expect(ctrl.$valid).toBe(false);
 
-      validatorResult = true;
-      ctrl.$validate();
+        validatorResult = true;
+        ctrl.$validate();
 
-      expect(ctrl.$valid).toBe(true);
-    });
+        expect(ctrl.$valid).toBe(true);
+      });
 
-    it('should always perform validations using the parsed model value', function() {
-      var captures;
-      ctrl.$validators.raw = function() {
-        captures = arguments;
-        return captures[0];
-      };
+      it('should pass the last parsed modelValue to the validators', function() {
+        ctrl.$parsers.push(function(modelValue) {
+          return modelValue + 'def';
+        });
 
-      ctrl.$parsers.push(function(value) {
-        return value.toUpperCase();
+        ctrl.$setViewValue('abc');
+
+        ctrl.$validators.test = function(modelValue, viewValue) {
+          return true;
+        };
+
+        spyOn(ctrl.$validators, 'test');
+
+        ctrl.$validate();
+
+        expect(ctrl.$validators.test).toHaveBeenCalledWith('abcdef', 'abc');
       });
 
-      ctrl.$setViewValue('my-value');
+      it('should set the model to undefined when it becomes invalid', function() {
+        var valid = true;
+        ctrl.$validators.test = function(modelValue, viewValue) {
+          return valid;
+        };
 
-      expect(captures).toEqual(['MY-VALUE', 'my-value']);
-    });
+        scope.$apply('value = "abc"');
+        expect(scope.value).toBe('abc');
 
-    it('should always perform validations using the formatted view value', function() {
-      var captures;
-      ctrl.$validators.raw = function() {
-        captures = arguments;
-        return captures[0];
-      };
+        valid = false;
+        ctrl.$validate();
 
-      ctrl.$formatters.push(function(value) {
-        return value + '...';
+        expect(scope.value).toBeUndefined();
+      });
+
+      it('should update the model when it becomes valid', function() {
+        var valid = true;
+        ctrl.$validators.test = function(modelValue, viewValue) {
+          return valid;
+        };
+
+        scope.$apply('value = "abc"');
+        expect(scope.value).toBe('abc');
+
+        valid = false;
+        ctrl.$validate();
+        expect(scope.value).toBeUndefined();
+
+        valid = true;
+        ctrl.$validate();
+        expect(scope.value).toBe('abc');
+      });
+
+      it('should not update the model when it is valid, but there is a parse error', function() {
+        ctrl.$parsers.push(function(modelValue) {
+          return undefined;
+        });
+
+        ctrl.$setViewValue('abc');
+        expect(ctrl.$error.parse).toBe(true);
+        expect(scope.value).toBeUndefined();
+
+        ctrl.$validators.test = function(modelValue, viewValue) {
+          return true;
+        };
+
+        ctrl.$validate();
+        expect(ctrl.$error).toEqual({parse: true});
+        expect(scope.value).toBeUndefined();
+      });
+
+      it('should not set an invalid model to undefined when validity is the same', function() {
+        ctrl.$validators.test = function() {
+          return false;
+        };
+
+        scope.$apply('value = "invalid"');
+        expect(ctrl.$valid).toBe(false);
+        expect(scope.value).toBe('invalid');
+
+        ctrl.$validate();
+        expect(ctrl.$valid).toBe(false);
+        expect(scope.value).toBe('invalid');
       });
 
-      scope.$apply('value = "matias"');
+      it('should not change a model that has a formatter', function() {
+        ctrl.$validators.test = function() {
+          return true;
+        };
+
+        ctrl.$formatters.push(function(modelValue) {
+          return 'xyz';
+        });
+
+        scope.$apply('value = "abc"');
+        expect(ctrl.$viewValue).toBe('xyz');
+
+        ctrl.$validate();
+        expect(scope.value).toBe('abc');
+      });
+
+      it('should not change a model that has a parser', function() {
+        ctrl.$validators.test = function() {
+          return true;
+        };
+
+        ctrl.$parsers.push(function(modelValue) {
+          return 'xyz';
+        });
+
+        scope.$apply('value = "abc"');
 
-      expect(captures).toEqual(['matias', 'matias...']);
+        ctrl.$validate();
+        expect(scope.value).toBe('abc');
+      });
     });
 
-    it('should only perform validations if the view value is different', function() {
-      var count = 0;
-      ctrl.$validators.countMe = function() {
-        count++;
-      };
+    describe('view -> model update', function() {
+      it('should always perform validations using the parsed model value', function() {
+        var captures;
+        ctrl.$validators.raw = function() {
+          captures = arguments;
+          return captures[0];
+        };
 
-      ctrl.$setViewValue('my-value');
-      expect(count).toBe(1);
+        ctrl.$parsers.push(function(value) {
+          return value.toUpperCase();
+        });
 
-      ctrl.$setViewValue('my-value');
-      expect(count).toBe(1);
+        ctrl.$setViewValue('my-value');
 
-      ctrl.$setViewValue('your-value');
-      expect(count).toBe(2);
+        expect(captures).toEqual(['MY-VALUE', 'my-value']);
+      });
+
+      it('should always perform validations using the formatted view value', function() {
+        var captures;
+        ctrl.$validators.raw = function() {
+          captures = arguments;
+          return captures[0];
+        };
+
+        ctrl.$formatters.push(function(value) {
+          return value + '...';
+        });
+
+        scope.$apply('value = "matias"');
+
+        expect(captures).toEqual(['matias', 'matias...']);
+      });
+
+      it('should only perform validations if the view value is different', function() {
+        var count = 0;
+        ctrl.$validators.countMe = function() {
+          count++;
+        };
+
+        ctrl.$setViewValue('my-value');
+        expect(count).toBe(1);
+
+        ctrl.$setViewValue('my-value');
+        expect(count).toBe(1);
+
+        ctrl.$setViewValue('your-value');
+        expect(count).toBe(2);
+      });
     });
 
     it('should perform validations twice each time the model value changes within a digest', function() {
@@ -934,6 +1052,7 @@ describe('NgModelController', function() {
   });
 });
 
+
 describe('ngModel', function() {
   var EMAIL_REGEXP = /^[a-z0-9!#$%&'*+\/=?^_`{|}~.-]+@[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/i;
 
@@ -1348,7 +1467,6 @@ describe('input', function() {
     expect(scope.form.$$renameControl).not.toHaveBeenCalled();
   });
 
-
   describe('compositionevents', function() {
     it('should not update the model between "compositionstart" and "compositionend" on non android', inject(function($sniffer) {
       $sniffer.android = false;
@@ -2311,6 +2429,14 @@ describe('input', function() {
       expect(inputElm).toBeValid();
       expect(scope.form.input.$error.minlength).not.toBe(true);
     });
+
+    it('should validate when the model is initalized as a number', function() {
+      scope.value = 12345;
+      compileInput('<input type="text" name="input" ng-model="value" minlength="3" />');
+      expect(scope.value).toBe(12345);
+      expect(scope.form.input.$error.minlength).toBeUndefined();
+    });
+
   });
 
 
@@ -2409,6 +2535,13 @@ describe('input', function() {
       expect(scope.value).toBe('12345');
     });
 
+    it('should validate when the model is initalized as a number', function() {
+      scope.value = 12345;
+      compileInput('<input type="text" name="input" ng-model="value" maxlength="10" />');
+      expect(scope.value).toBe(12345);
+      expect(scope.form.input.$error.maxlength).toBeUndefined();
+    });
+
   });