From 4644c5840fc6ec84203ad854bde68f4883ff5aa3 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 21 Nov 2014 19:47:01 +0000 Subject: [PATCH 1/4] revert: fix(input): always pass in the model value to `ctrl.$isEmpty` This commit tried to create consistency by ensuring that `$isEmpty` is not called on both model and view values but it chose to only use `$modelValue`, which is not actually correct. `$isEmpty` is designed to compute whether the `$viewValue` is empty. In practice this is the only part of the parse/format system that the directive has control over. We can't rely on the `$modelValue` being in any particular format since other directives can add in their own formatters and parsers to completely change this. (reverted from commit 3e51b84bc19f7e6acc61cb536ddcdbfed307c831) --- src/ng/directive/input.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index afa8347e23c6..cf26c32d041c 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1039,7 +1039,7 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) { element.on('change', listener); ctrl.$render = function() { - element.val(ctrl.$isEmpty(ctrl.$modelValue) ? '' : ctrl.$viewValue); + element.val(ctrl.$isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue); }; } @@ -1274,7 +1274,8 @@ function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) { stringBasedInputType(ctrl); ctrl.$$parserName = 'url'; - ctrl.$validators.url = function(value) { + ctrl.$validators.url = function(modelValue, viewValue) { + var value = modelValue || viewValue; return ctrl.$isEmpty(value) || URL_REGEXP.test(value); }; } @@ -1286,7 +1287,8 @@ function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) { stringBasedInputType(ctrl); ctrl.$$parserName = 'email'; - ctrl.$validators.email = function(value) { + ctrl.$validators.email = function(modelValue, viewValue) { + var value = modelValue || viewValue; return ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value); }; } @@ -1340,7 +1342,7 @@ function checkboxInputType(scope, element, attr, ctrl, $sniffer, $browser, $filt element[0].checked = ctrl.$viewValue; }; - // Override the standard `$isEmpty` because an empty checkbox is never equal to the trueValue + // Override the standard `$isEmpty` because a value of `false` means empty in a checkbox. ctrl.$isEmpty = function(value) { return value !== trueValue; }; @@ -1819,7 +1821,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * default. The `checkboxInputType` directive does this because in its case a value of `false` * implies empty. * - * @param {*} value Model value to check. + * @param {*} value Reference to check. * @returns {boolean} True if `value` is empty. */ this.$isEmpty = function(value) { @@ -2647,8 +2649,8 @@ var requiredDirective = function() { if (!ctrl) return; attr.required = true; // force truthy in case we are on non input element - ctrl.$validators.required = function(value) { - return !attr.required || !ctrl.$isEmpty(value); + ctrl.$validators.required = function(modelValue, viewValue) { + return !attr.required || !ctrl.$isEmpty(viewValue); }; attr.$observe('required', function() { @@ -2723,7 +2725,7 @@ var minlengthDirective = function() { ctrl.$validate(); }); ctrl.$validators.minlength = function(modelValue, viewValue) { - return ctrl.$isEmpty(modelValue) || viewValue.length >= minlength; + return ctrl.$isEmpty(viewValue) || viewValue.length >= minlength; }; } }; From 40406e2f22713efbd37ef3eff408339727cb62d9 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 21 Nov 2014 19:47:43 +0000 Subject: [PATCH 2/4] fix(input[date]): do not use `$isEmpty` to check the model validity --- src/ng/directive/input.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index cf26c32d041c..95497a3c7aba 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1149,10 +1149,10 @@ function createDateInputType(type, regexp, parseDate, format) { }); ctrl.$formatters.push(function(value) { - if (!ctrl.$isEmpty(value)) { - if (!isDate(value)) { - throw $ngModelMinErr('datefmt', 'Expected `{0}` to be a date', value); - } + if (value && !isDate(value)) { + throw $ngModelMinErr('datefmt', 'Expected `{0}` to be a date', value); + } + if (isValidDate(value)) { previousDate = value; if (previousDate && timezone === 'UTC') { var timezoneOffset = 60000 * previousDate.getTimezoneOffset(); @@ -1161,14 +1161,14 @@ function createDateInputType(type, regexp, parseDate, format) { return $filter('date')(value, format, timezone); } else { previousDate = null; + return ''; } - return ''; }); if (isDefined(attr.min) || attr.ngMin) { var minVal; ctrl.$validators.min = function(value) { - return ctrl.$isEmpty(value) || isUndefined(minVal) || parseDate(value) >= minVal; + return !isValidDate(value) || isUndefined(minVal) || parseDate(value) >= minVal; }; attr.$observe('min', function(val) { minVal = parseObservedDateValue(val); @@ -1179,18 +1179,18 @@ function createDateInputType(type, regexp, parseDate, format) { if (isDefined(attr.max) || attr.ngMax) { var maxVal; ctrl.$validators.max = function(value) { - return ctrl.$isEmpty(value) || isUndefined(maxVal) || parseDate(value) <= maxVal; + return !isValidDate(value) || isUndefined(maxVal) || parseDate(value) <= maxVal; }; attr.$observe('max', function(val) { maxVal = parseObservedDateValue(val); ctrl.$validate(); }); } - // Override the standard $isEmpty to detect invalid dates as well - ctrl.$isEmpty = function(value) { + + function isValidDate(value) { // Invalid Date: getTime() returns NaN - return !value || (value.getTime && value.getTime() !== value.getTime()); - }; + return value && !(value.getTime && value.getTime() !== value.getTime()); + } function parseObservedDateValue(val) { return isDefined(val) ? (isDate(val) ? val : parseDate(val)) : undefined; From 8692f87a4689fa0dd3640f4dcab5c6b6f960489b Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Fri, 21 Nov 2014 19:47:43 +0000 Subject: [PATCH 3/4] fix(input): set ngTrueValue on required checkbox Fixes #5164 --- src/ng/directive/input.js | 6 ++++-- test/ng/directive/inputSpec.js | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 95497a3c7aba..07ef8ad55ae4 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1342,9 +1342,11 @@ function checkboxInputType(scope, element, attr, ctrl, $sniffer, $browser, $filt element[0].checked = ctrl.$viewValue; }; - // Override the standard `$isEmpty` because a value of `false` means empty in a checkbox. + // Override the standard `$isEmpty` because the $viewValue of an empty checkbox is always set to `false` + // This is because of the parser below, which compares the `$modelValue` with `trueValue` to convert + // it to a boolean. ctrl.$isEmpty = function(value) { - return value !== trueValue; + return value === false; }; ctrl.$formatters.push(function(value) { diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index b2ab27ac9469..e579eb0f8db3 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -2290,6 +2290,22 @@ describe('input', function() { }); + it('should render the $viewValue when $modelValue is empty', function() { + compileInput(''); + + var ctrl = inputElm.controller('ngModel'); + + ctrl.$modelValue = null; + + expect(ctrl.$isEmpty(ctrl.$modelValue)).toBe(true); + + ctrl.$viewValue = 'abc'; + ctrl.$render(); + + expect(inputElm.val()).toBe('abc'); + }); + + describe('pattern', function() { it('should validate in-lined pattern', function() { From 42d09f1772aeb15b3838fd2f18ff8af674cd6777 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sun, 23 Nov 2014 15:24:36 +0000 Subject: [PATCH 4/4] docs(NgModelController): clarify the value parameter for $isEmpty --- src/ng/directive/input.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 07ef8ad55ae4..9644ab21a0d7 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1814,17 +1814,18 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * @name ngModel.NgModelController#$isEmpty * * @description - * This is called when we need to determine if the value of the input is empty. + * This is called when we need to determine if the value of an input is empty. * * For instance, the required directive does this to work out if the input has data or not. + * * The default `$isEmpty` function checks whether the value is `undefined`, `''`, `null` or `NaN`. * * You can override this for input directives whose concept of being empty is different to the * default. The `checkboxInputType` directive does this because in its case a value of `false` * implies empty. * - * @param {*} value Reference to check. - * @returns {boolean} True if `value` is empty. + * @param {*} value The value of the input to check for emptiness. + * @returns {boolean} True if `value` is "empty". */ this.$isEmpty = function(value) { return isUndefined(value) || value === '' || value === null || value !== value;