Skip to content

Commit

Permalink
fix(core): drop the toBoolean function
Browse files Browse the repository at this point in the history
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 treated as falsy by the
expression parser; there are six of them: false, null, undefined, NaN, o and "".

Fixes angular#3969
Fixes angular#4277
  • Loading branch information
mgol committed Jun 23, 2014
1 parent e8e0750 commit 1475da5
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 34 deletions.
1 change: 0 additions & 1 deletion src/.jshintrc
Expand Up @@ -85,7 +85,6 @@
"toJsonReplacer": false,
"toJson": false,
"fromJson": false,
"toBoolean": false,
"startingTag": false,
"tryDecodeURIComponent": false,
"parseKeyValue": false,
Expand Down
13 changes: 0 additions & 13 deletions src/Angular.js
Expand Up @@ -66,7 +66,6 @@
-toJsonReplacer,
-toJson,
-fromJson,
-toBoolean,
-startingTag,
-tryDecodeURIComponent,
-parseKeyValue,
Expand Down Expand Up @@ -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.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/input.js
Expand Up @@ -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. <input ng-model="foo" ng-trim="false">
if (toBoolean(attr.ngTrim || 'T')) {
if (!attr.ngTrim || attr.ngTrim !== 'false') {
value = trim(value);
}

Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/ngIf.js
Expand Up @@ -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;
Expand Down
22 changes: 6 additions & 16 deletions src/ng/directive/ngShowHide.js
Expand Up @@ -19,15 +19,10 @@
* <div ng-show="myValue" class="ng-hide"></div>
* ```
*
* 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.
*
* <div class="alert alert-warning">
* **Note:** Here is a list of values that ngShow will consider as a falsy value (case insensitive):<br />
* "f" / "0" / "false" / "no" / "n" / "[]"
* </div>
*
* ## 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
Expand Down Expand Up @@ -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');
});
};
}];
Expand All @@ -188,15 +183,10 @@ var ngShowDirective = ['$animate', function($animate) {
* <div ng-hide="myValue"></div>
* ```
*
* 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.
*
* <div class="alert alert-warning">
* **Note:** Here is a list of values that ngHide will consider as a falsy value (case insensitive):<br />
* "f" / "0" / "false" / "no" / "n" / "[]"
* </div>
*
* ## 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
Expand Down Expand Up @@ -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');
});
};
}];
2 changes: 1 addition & 1 deletion src/ng/filter/orderBy.js
Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion test/.jshintrc
Expand Up @@ -85,7 +85,6 @@
"toJsonReplacer": false,
"toJson": false,
"fromJson": false,
"toBoolean": false,
"startingTag": false,
"tryDecodeURIComponent": false,
"parseKeyValue": false,
Expand Down
20 changes: 20 additions & 0 deletions test/BinderSpec.js
Expand Up @@ -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 = '';
Expand All @@ -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 = '';
Expand Down

0 comments on commit 1475da5

Please sign in to comment.