Skip to content
Permalink
Browse files

fix(ngModelController): always use the most recent viewValue for vali…

…dation

This fixes issues where a parser calls $setViewValue. This is a common
strategy for manipulating the $viewValue while the user is entering
data into an input field.

When the $viewValue was changed inside the parser, the new viewValue
would be committed, parsed and used for validation. The original parser
however would run after that and pass the original (outdated) viewValue
on to the validators, which could cause false positives, e.g. for
minlength.

Fixes #10126
Fixes #10299
  • Loading branch information
Narretz committed Dec 2, 2014
1 parent 0c4f9fa commit 2d6a0a1dc1e7125cab2e30244e35e97e11802843
Showing with 55 additions and 1 deletion.
  1. +5 −1 src/ng/directive/input.js
  2. +50 −0 test/ng/directive/inputSpec.js
@@ -2196,11 +2196,15 @@ 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();
}
ctrl.$$runValidators(parserValid, modelValue, viewValue, function(allValid) {

// 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) {
if (!allowInvalid) {
// Note: Don't check ctrl.$valid here, as we could have
// external validators (e.g. calculated on the server),
@@ -1066,6 +1066,56 @@ describe('NgModelController', function() {

dealoc(element);
}));

it('should always use the most recent $viewValue for validation', function() {
ctrl.$parsers.push(function(value) {
if (value && value.substr(-1) === 'b') {
value = 'a';
ctrl.$setViewValue(value);
ctrl.$render();
}

return value;
});

ctrl.$validators.mock = function(modelValue) {
return true;
};

spyOn(ctrl.$validators, 'mock').andCallThrough();

ctrl.$setViewValue('ab');

expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'a');
expect(ctrl.$validators.mock.calls.length).toEqual(2);
});

it('should validate even if the modelValue did not change', function() {
ctrl.$parsers.push(function(value) {
if (value && value.substr(-1) === 'b') {
value = 'a';
}

return value;
});

ctrl.$validators.mock = function(modelValue) {
return true;
};

spyOn(ctrl.$validators, 'mock').andCallThrough();

ctrl.$setViewValue('a');

expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'a');
expect(ctrl.$validators.mock.calls.length).toEqual(1);

ctrl.$setViewValue('ab');

expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'ab');
expect(ctrl.$validators.mock.calls.length).toEqual(2);
});

});
});

0 comments on commit 2d6a0a1

Please sign in to comment.
You can’t perform that action at this time.