diff --git a/src/domUtils/CSSProperty.js b/src/domUtils/CSSProperty.js index 389fccf3e7..95586d5a7c 100644 --- a/src/domUtils/CSSProperty.js +++ b/src/domUtils/CSSProperty.js @@ -19,20 +19,19 @@ "use strict"; /** - * CSS properties for which we do not append "px". + * CSS properties which accept numbers but are not in units of "px". */ -var isNumber = { +var isUnitlessNumber = { fillOpacity: true, fontWeight: true, opacity: true, orphans: true, - textDecoration: true, zIndex: true, zoom: true }; var CSSProperty = { - isNumber: isNumber + isUnitlessNumber: isUnitlessNumber }; module.exports = CSSProperty; diff --git a/src/domUtils/CSSPropertyOperations.js b/src/domUtils/CSSPropertyOperations.js index 76939a2b00..4c67a4cbe8 100644 --- a/src/domUtils/CSSPropertyOperations.js +++ b/src/domUtils/CSSPropertyOperations.js @@ -42,7 +42,7 @@ var CSSPropertyOperations = { * Undefined values are ignored so that declarative programming is easier. * * @param {object} styles - * @return {string} + * @return {?string} */ createMarkupForStyles: function(styles) { var serialized = ''; @@ -51,12 +51,12 @@ var CSSPropertyOperations = { continue; } var styleValue = styles[styleName]; - if (typeof styleValue !== 'undefined') { + if (styleValue != null) { serialized += processStyleName(styleName) + ':'; serialized += dangerousStyleValue(styleName, styleValue) + ';'; } } - return serialized; + return serialized || null; }, /** diff --git a/src/domUtils/__tests__/CSSPropertyOperations-test.js b/src/domUtils/__tests__/CSSPropertyOperations-test.js index 8978fa1f2c..3918d801af 100644 --- a/src/domUtils/__tests__/CSSPropertyOperations-test.js +++ b/src/domUtils/__tests__/CSSPropertyOperations-test.js @@ -19,6 +19,8 @@ "use strict"; +var React = require('React'); + describe('CSSPropertyOperations', function() { var CSSPropertyOperations; @@ -41,12 +43,49 @@ describe('CSSPropertyOperations', function() { })).toBe('display:none;'); }); + it('should ignore null styles', function() { + expect(CSSPropertyOperations.createMarkupForStyles({ + backgroundColor: null, + display: 'none' + })).toBe('display:none;'); + }); + + it('should return null for no styles', function() { + expect(CSSPropertyOperations.createMarkupForStyles({ + backgroundColor: null, + display: null + })).toBe(null); + }); + it('should automatically append `px` to relevant styles', function() { expect(CSSPropertyOperations.createMarkupForStyles({ + left: 0, margin: 16, opacity: 0.5, padding: '4px' - })).toBe('margin:16px;opacity:0.5;padding:4px;'); + })).toBe('left:0;margin:16px;opacity:0.5;padding:4px;'); + }); + + it('should set style attribute when styles exist', function() { + var styles = { + backgroundColor: '#000', + display: 'none' + }; + var div =
; + var root = document.createElement('div'); + React.renderComponent(div, root); + expect(/style=".*"/.test(root.innerHTML)).toBe(true); + }); + + it('should not set style attribute when no styles exist', function() { + var styles = { + backgroundColor: null, + display: null + }; + var div = ; + var root = document.createElement('div'); + React.renderComponent(div, root); + expect(/style=".*"/.test(root.innerHTML)).toBe(false); }); }); diff --git a/src/domUtils/dangerousStyleValue.js b/src/domUtils/dangerousStyleValue.js index aef5c1d555..9c12b12ab3 100644 --- a/src/domUtils/dangerousStyleValue.js +++ b/src/domUtils/dangerousStyleValue.js @@ -23,7 +23,8 @@ var CSSProperty = require('CSSProperty'); /** * Convert a value into the proper css writable value. The `styleName` name - * name should be logical (no hyphens), as specified in `CSSProperty.isNumber`. + * name should be logical (no hyphens), as specified + * in `CSSProperty.isUnitlessNumber`. * * @param {string} styleName CSS property name such as `topMargin`. * @param {*} value CSS property value such as `10px`. @@ -39,13 +40,18 @@ function dangerousStyleValue(styleName, value) { // This is not an XSS hole but instead a potential CSS injection issue // which has lead to a greater discussion about how we're going to // trust URLs moving forward. See #2115901 - if (value === null || value === false || value === true || value === '') { + + var isEmpty = value == null || typeof value === 'boolean' || value === ''; + if (isEmpty) { return ''; } - if (isNaN(value)) { - return !value ? '' : '' + value; + + var isNonNumeric = isNaN(value); + if (isNonNumeric || value === 0 || CSSProperty.isUnitlessNumber[styleName]) { + return '' + value; // cast to string } - return CSSProperty.isNumber[styleName] ? '' + value : (value + 'px'); + + return value + 'px'; } module.exports = dangerousStyleValue;