From c4ff1660b9da76bc95d007168e3b620cf54facd0 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 2 Dec 2014 21:27:11 +0100 Subject: [PATCH] fix(ngModelController): always use the most recent viewValue for validation 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 --- src/ng/directive/input.js | 6 +++- test/ng/directive/inputSpec.js | 50 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 9644ab21a0d7..32a045d92914 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -2200,11 +2200,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), diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index e579eb0f8db3..d089f412cf8d 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -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); + }); + }); });