wip: fix(ngModel): don't update when neither validity nor modelValue changed

src/ng/directive/input.js

@@ -1956,13 +1956,40 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
    *
    * @description
    * 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 ngModelOptions.allowInvalid
+   * is `true`. If the validity changes to valid, it will set the model to the last available valid
+   * modelValue.
    */
   this.$validate = function() {
     // ignore $validate before model is initialized
     if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
       return;
     }
-    this.$$parseAndValidate();
+
+    var viewValue = ctrl.$$lastCommittedViewValue;
+    var modelValue = ctrl.$$rawModelValue;
+
+    var prevValid = ctrl.$valid;
+    var prevModelValue = ctrl.$modelValue;
+
+    var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
+
+    ctrl.$$runValidators(undefined, 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) {
@@ -2110,6 +2137,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();
@@ -2234,7 +2262,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

@@ -934,6 +934,209 @@ describe('NgModelController', function() {
   });
 });
 
+describe('initialization', function() {
+  var formElm, inputElm, ctrl, scope, $compile, $sniffer, $compileProvider, changeInputValueTo;
+
+  function compileInput(inputHtml) {
+    inputElm = jqLite(inputHtml);
+    formElm = jqLite('<form name="form"></form>');
+    formElm.append(inputElm);
+    $compile(formElm)(scope);
+    ctrl = inputElm.controller('ngModel');
+    scope.$digest();
+  }
+
+  function addValidator(validity, shouldObserve) {
+    if (!isDefined(shouldObserve)) {
+      shouldObserve = true;
+    }
+
+    $compileProvider.directive('obs', function() {
+      return {
+        require: 'ngModel',
+        link: function(scope, element, attrs, ngModelCtrl) {
+
+          ngModelCtrl.$validators.obs = isFunction(validity) ? validity : function(value) {
+            return validity;
+          };
+
+          if (shouldObserve) {
+            attrs.$observe('obs', function() {
+              ngModelCtrl.$validate();
+            });
+          }
+        }
+      };
+
+    });
+  }
+
+  function addFormatter(formatFunction) {
+    $compileProvider.directive('format', function() {
+      return {
+        require: 'ngModel',
+        link: function(scope, element, attrs, ctrl) {
+
+          ctrl.$formatters.push(formatFunction);
+        }
+      };
+
+    });
+  }
+
+  function addParser(parseFunction) {
+    $compileProvider.directive('parse', function() {
+      return {
+        require: 'ngModel',
+        link: function(scope, element, attrs, ctrl) {
+
+          ctrl.$parsers.push(parseFunction);
+        }
+      };
+
+    });
+  }
+
+  beforeEach(module(function(_$compileProvider_) {
+    $compileProvider = _$compileProvider_;
+  }));
+
+  beforeEach(inject(function(_$compile_, _$rootScope_, _$sniffer_) {
+    $compile = _$compile_;
+    $sniffer = _$sniffer_;
+    scope = _$rootScope_;
+
+    changeInputValueTo = function(value) {
+      inputElm.val(value);
+      browserTrigger(inputElm, $sniffer.hasEvent('input') ? 'input' : 'change');
+    };
+  }));
+
+  afterEach(function() {
+    dealoc(formElm);
+  });
+
+  // https://github.com/angular/angular.js/issues/9959
+  it('should not change model of type number to string with validator using observer', function() {
+    addValidator(true);
+    scope.value = 12345;
+    scope.attr = 'mock';
+    scope.ngChangeSpy = jasmine.createSpy();
+
+    compileInput('<input type="text" name="input" ng-model="value"' +
+      'ng-change="ngChangeSpy()" obs="{{attr}}" />');
+
+    expect(scope.value).toBe(12345);
+    expect(scope.ngChangeSpy).not.toHaveBeenCalled();
+  });
+
+  //https://github.com/angular/angular.js/issues/9063
+  it('should not set a null model that is invalid to undefined', function() {
+    addValidator(false);
+    scope.value = null;
+    scope.required = true;
+    compileInput('<input type="text" name="textInput" ng-model="value"' +
+      'ng-required="required" obs="{{attr}}" />');
+
+    expect(inputElm).toBeInvalid();
+    expect(scope.value).toBe(null);
+    expect(scope.form.textInput.$error.obs).toBeTruthy();
+  });
+
+  //https://github.com/angular/angular.js/issues/9996
+  it('should not change an undefined model that uses ng-required and formatters and parsers', function() {
+    addParser(function(viewValue) {
+      return null;
+    });
+    addFormatter(function(modelValue) {
+      return '';
+    });
+
+    scope.ngChangeSpy = jasmine.createSpy();
+    compileInput('<input type="text" parse format name="textInput" ng-model="value"' +
+      'ng-required="undefinedProp" ng-change="ngChangeSpy()" />');
+
+    expect(inputElm).toBeValid();
+    expect(scope.value).toBeUndefined();
+    expect(scope.ngChangeSpy).not.toHaveBeenCalled();
+  });
+
+  // https://github.com/angular/angular.js/issues/10025
+  it('should not change a model that uses custom $formatters and $parsers', function() {
+    addValidator(true);
+    addFormatter(function(modelValue) {
+      return 'abc';
+    });
+    addParser(function(viewValue) {
+      return 'xyz';
+    });
+    scope.value = 'abc';
+    scope.attr = 'mock';
+    compileInput('<input type="text" parse format name="textInput" ng-model="value"' +
+      'obs="{{attr}}" />');
+
+    expect(inputElm).toBeValid();
+    expect(scope.value).toBe('abc');
+  });
+
+  describe('$validate', function() {
+
+    // Sanity test: since a parse error sets the modelValue to undefined, the
+    // $$rawModelValue will always be undefined, hence $validate does not have
+    // a 'good' value to update
+    it('should not update a model that has a parse error', function() {
+      scope.value = 'abc';
+      addParser(function() {
+        return undefined;
+      });
+
+      addValidator(true, false);
+
+      compileInput('<input type="text" name="textInput" obs parse ng-model="value"/>');
+      expect(inputElm).toBeValid();
+      expect(scope.value).toBe('abc');
+
+      changeInputValueTo('xyz');
+      expect(inputElm).toBeInvalid();
+      expect(scope.value).toBeUndefined();
+      expect(ctrl.$error.parse).toBe(true);
+
+      ctrl.$validate();
+      expect(inputElm).toBeInvalid();
+      expect(scope.value).toBeUndefined();
+    });
+
+    it('should restore the last valid modelValue when a validator becomes valid', function() {
+      scope.value = 'abc';
+      scope.count = 0;
+
+      addValidator(function() {
+        scope.count++;
+        dump('count', scope.count);
+        return scope.count === 1 ? true : scope.count === 2 ? false : true;
+      });
+
+      compileInput('<input type="text" name="textInput" obs ng-model="value"/>');
+      expect(inputElm).toBeValid();
+      expect(scope.value).toBe('abc');
+      expect(ctrl.$viewValue).toBe('abc');
+
+      ctrl.$validate();
+      scope.$digest();
+      expect(inputElm).toBeInvalid();
+      expect(scope.value).toBeUndefined();
+      expect(ctrl.$viewValue).toBe('abc');
+
+      ctrl.$validate();
+      scope.$digest();
+      expect(inputElm).toBeValid();
+      expect(scope.value).toBe('abc');
+    });
+
+
+  });
+});
+
 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,6 +1551,16 @@ describe('input', function() {
     expect(scope.form.$$renameControl).not.toHaveBeenCalled();
   });
 
+  it('should not invoke viewChangeListeners before input is touched', function() {
+    scope.value = 1;
+    var change = scope.change = jasmine.createSpy('change');
+    var element = $compile('<div><div ng-repeat="i in [1]">' +
+      '<input type="text" ng-model="value" maxlength="1" ng-change="change()" />' +
+      '</div></div>')(scope);
+    scope.$digest();
+    expect(change).not.toHaveBeenCalled();
+    dealoc(element);
+  });
 
   describe('compositionevents', function() {
     it('should not update the model between "compositionstart" and "compositionend" on non android', inject(function($sniffer) {
@@ -2264,6 +2477,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();
+    });
+
   });
 
 
@@ -2362,6 +2583,14 @@ describe('input', function() {
       expect(scope.value).toBe('12345');
     });
 
+    // This works both for string formatter and toString() in validator
+    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();
+    });
+
   });