From 9e5ac74c3ea1fafbd8242cc78215733f1dd87bfa Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 27 Apr 2019 17:11:21 +0300 Subject: [PATCH] fix(ngStyle): correctly remove old style when new style value is invalid Since d6098eeb1, old styles were not removed if `newStyles` specified an invalid value for the style (e.g. `false`). The assumption was that the new style would overwrite the old style value, but using an invalid value made browsers ignore the new value and thus keep the old style. This would typically happen when guarding a style with a boolean flag; e.g.: `ng-style="{backgroundColor: isError && 'red'}"` This commit essentially revers commit d6098eeb1, whose main purpose was to work around jquery/jquery#4185. The jQuery issue has been fixed in 3.4.0, so that should not be a problem any more. Fixes #16860 --- src/ng/directive/ngStyle.js | 9 +-------- test/ng/directive/ngStyleSpec.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/ngStyle.js b/src/ng/directive/ngStyle.js index f1d3f66d90f0..c2f469a3e044 100644 --- a/src/ng/directive/ngStyle.js +++ b/src/ng/directive/ngStyle.js @@ -54,14 +54,7 @@ var ngStyleDirective = ngDirective(function(scope, element, attr) { scope.$watchCollection(attr.ngStyle, function ngStyleWatchAction(newStyles, oldStyles) { if (oldStyles && (newStyles !== oldStyles)) { - if (!newStyles) { - newStyles = {}; - } - forEach(oldStyles, function(val, style) { - if (newStyles[style] == null) { - newStyles[style] = ''; - } - }); + forEach(oldStyles, function(val, style) { element.css(style, ''); }); } if (newStyles) element.css(newStyles); }); diff --git a/test/ng/directive/ngStyleSpec.js b/test/ng/directive/ngStyleSpec.js index 51dd1c3fcb0c..0a3aedea4190 100644 --- a/test/ng/directive/ngStyleSpec.js +++ b/test/ng/directive/ngStyleSpec.js @@ -143,6 +143,17 @@ describe('ngStyle', function() { expect(element.css(postCompStyle)).not.toBe('99px'); }); + it('should clear style when the value is false', function() { + scope.styleObj = {'height': '99px', 'width': '88px'}; + scope.$apply(); + expect(element.css(preCompStyle)).toBe('88px'); + expect(element.css(postCompStyle)).toBe('99px'); + scope.styleObj = {'height': false, 'width': false}; + scope.$apply(); + expect(element.css(preCompStyle)).not.toBe('88px'); + expect(element.css(postCompStyle)).not.toBe('99px'); + }); + it('should set style when the value is zero', function() { scope.styleObj = {'height': '99px', 'width': '88px'}; scope.$apply();