From bbe52dd097e5e158cde40bb1b1e1bc3596a15349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski?= Date: Tue, 24 Jun 2014 00:07:30 +0200 Subject: [PATCH] fix(core): drop the toBoolean function So far Angular have used the toBoolean function to decide if the parsed value is truthy. The function made more values falsy than regular JavaScript would, e.g. strings 'f' and 'no' were both treated as falsy. This creates suble bugs when backend sends a non-empty string with one of these values and something suddenly hides in the application BREAKING CHANGE: values 'f', '0', 'false', 'no', 'n', '[]' are no longer treated as falsy. Only JavaScript falsy values are now treated as falsy by the expression parser; there are six of them: false, null, undefined, NaN, o and "". Fixes #3969 Fixes #4277 --- src/.jshintrc | 1 - src/Angular.js | 13 ------------- src/ng/directive/input.js | 2 +- src/ng/directive/ngIf.js | 2 +- src/ng/directive/ngShowHide.js | 22 ++++++---------------- src/ng/filter/orderBy.js | 2 +- test/.jshintrc | 1 - test/BinderSpec.js | 20 ++++++++++++++++++++ 8 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/.jshintrc b/src/.jshintrc index 0a9094787c39..3350af2e47c4 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -85,7 +85,6 @@ "toJsonReplacer": false, "toJson": false, "fromJson": false, - "toBoolean": false, "startingTag": false, "tryDecodeURIComponent": false, "parseKeyValue": false, diff --git a/src/Angular.js b/src/Angular.js index 711483a26fd3..3fee136d73df 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -66,7 +66,6 @@ -toJsonReplacer, -toJson, -fromJson, - -toBoolean, -startingTag, -tryDecodeURIComponent, -parseKeyValue, @@ -1028,18 +1027,6 @@ function fromJson(json) { } -function toBoolean(value) { - if (typeof value === 'function') { - value = true; - } else if (value && value.length !== 0) { - var v = lowercase("" + value); - value = !(v == 'f' || v == '0' || v == 'false' || v == 'no' || v == 'n' || v == '[]'); - } else { - value = false; - } - return value; -} - /** * @returns {string} Returns the string representation of the element. */ diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 039f6abd77c1..fd49f8e5ed4e 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -917,7 +917,7 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { // By default we will trim the value // If the attribute ng-trim exists we will avoid trimming // e.g. - if (toBoolean(attr.ngTrim || 'T')) { + if (!attr.ngTrim || attr.ngTrim !== 'false') { value = trim(value); } diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js index f75674039a1d..12cff12340b1 100644 --- a/src/ng/directive/ngIf.js +++ b/src/ng/directive/ngIf.js @@ -87,7 +87,7 @@ var ngIfDirective = ['$animate', function($animate) { var block, childScope, previousElements; $scope.$watch($attr.ngIf, function ngIfWatchAction(value) { - if (toBoolean(value)) { + if (!!value) { if (!childScope) { $transclude(function (clone, newScope) { childScope = newScope; diff --git a/src/ng/directive/ngShowHide.js b/src/ng/directive/ngShowHide.js index a04357eca07d..3c756e6ceaca 100644 --- a/src/ng/directive/ngShowHide.js +++ b/src/ng/directive/ngShowHide.js @@ -19,15 +19,10 @@ *
* ``` * - * When the ngShow expression evaluates to false then the ng-hide CSS class is added to the class attribute - * on the element causing it to become hidden. When true, the ng-hide CSS class is removed + * When the ngShow expression evaluates to a falsy value then the ng-hide CSS class is added to the class + * attribute on the element causing it to become hidden. When true, the ng-hide CSS class is removed * from the element causing the element not to appear hidden. * - *
- * **Note:** Here is a list of values that ngShow will consider as a falsy value (case insensitive):
- * "f" / "0" / "false" / "no" / "n" / "[]" - *
- * * ## Why is !important used? * * You may be wondering why !important is used for the .ng-hide CSS class. This is because the `.ng-hide` selector @@ -163,7 +158,7 @@ var ngShowDirective = ['$animate', function($animate) { return function(scope, element, attr) { scope.$watch(attr.ngShow, function ngShowWatchAction(value){ - $animate[toBoolean(value) ? 'removeClass' : 'addClass'](element, 'ng-hide'); + $animate[!!value ? 'removeClass' : 'addClass'](element, 'ng-hide'); }); }; }]; @@ -188,15 +183,10 @@ var ngShowDirective = ['$animate', function($animate) { *
* ``` * - * When the ngHide expression evaluates to true then the .ng-hide CSS class is added to the class attribute - * on the element causing it to become hidden. When false, the ng-hide CSS class is removed + * When the ngHide expression evaluates to a truthy value then the .ng-hide CSS class is added to the class + * attribute on the element causing it to become hidden. When false, the ng-hide CSS class is removed * from the element causing the element not to appear hidden. * - *
- * **Note:** Here is a list of values that ngHide will consider as a falsy value (case insensitive):
- * "f" / "0" / "false" / "no" / "n" / "[]" - *
- * * ## Why is !important used? * * You may be wondering why !important is used for the .ng-hide CSS class. This is because the `.ng-hide` selector @@ -319,7 +309,7 @@ var ngShowDirective = ['$animate', function($animate) { var ngHideDirective = ['$animate', function($animate) { return function(scope, element, attr) { scope.$watch(attr.ngHide, function ngHideWatchAction(value){ - $animate[toBoolean(value) ? 'addClass' : 'removeClass'](element, 'ng-hide'); + $animate[!!value ? 'addClass' : 'removeClass'](element, 'ng-hide'); }); }; }]; diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index c20add53dc5d..4efd9e19d9a6 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -144,7 +144,7 @@ function orderByFilter($parse){ return 0; } function reverseComparator(comp, descending) { - return toBoolean(descending) + return descending ? function(a,b){return comp(b,a);} : comp; } diff --git a/test/.jshintrc b/test/.jshintrc index fe91f7f29e2b..96560e0a82a8 100644 --- a/test/.jshintrc +++ b/test/.jshintrc @@ -85,7 +85,6 @@ "toJsonReplacer": false, "toJson": false, "fromJson": false, - "toBoolean": false, "startingTag": false, "tryDecodeURIComponent": false, "parseKeyValue": false, diff --git a/test/BinderSpec.js b/test/BinderSpec.js index 57f529b0a520..23519fe8fc17 100644 --- a/test/BinderSpec.js +++ b/test/BinderSpec.js @@ -248,6 +248,16 @@ describe('Binder', function() { $rootScope.hidden = 'false'; $rootScope.$apply(); + assertHidden(element); + + $rootScope.hidden = 0; + $rootScope.$apply(); + + assertVisible(element); + + $rootScope.hidden = false; + $rootScope.$apply(); + assertVisible(element); $rootScope.hidden = ''; @@ -267,6 +277,16 @@ describe('Binder', function() { $rootScope.show = 'false'; $rootScope.$apply(); + assertVisible(element); + + $rootScope.show = false; + $rootScope.$apply(); + + assertHidden(element); + + $rootScope.show = false; + $rootScope.$apply(); + assertHidden(element); $rootScope.show = '';