From 00a9c2e5f4c855382435cec6b3908eb9bd5a53b7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski-Owczarek?= Date: Mon, 23 Apr 2018 21:00:14 +0200 Subject: [PATCH] CSS: Don't automatically add "px" to properties with a few exceptions Fixes gh-2795 Closes gh-4055 Ref gh-4009 --- src/css.js | 35 +++---------------- src/css/adjustCSS.js | 7 ++-- src/css/isAutoPx.js | 41 ++++++++++++++++++++++ src/effects/Tween.js | 5 +-- test/unit/css.js | 82 ++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 133 insertions(+), 37 deletions(-) create mode 100644 src/css/isAutoPx.js diff --git a/src/css.js b/src/css.js index ac4c66e87..d6354a310 100644 --- a/src/css.js +++ b/src/css.js @@ -5,6 +5,7 @@ define( [ "./var/rcssNum", "./css/var/rnumnonpx", "./css/var/cssExpand", + "./css/isAutoPx", "./css/var/getStyles", "./css/var/swap", "./css/curCSS", @@ -16,7 +17,7 @@ define( [ "./core/init", "./core/ready", "./selector" // contains -], function( jQuery, access, camelCase, rcssNum, rnumnonpx, cssExpand, +], function( jQuery, access, camelCase, rcssNum, rnumnonpx, cssExpand, isAutoPx, getStyles, swap, curCSS, adjustCSS, addGetHookIf, support, finalPropName ) { "use strict"; @@ -198,30 +199,6 @@ jQuery.extend( { } }, - // Don't automatically add "px" to these possibly-unitless properties - cssNumber: { - "animationIterationCount": true, - "columnCount": true, - "fillOpacity": true, - "flexGrow": true, - "flexShrink": true, - "fontWeight": true, - "gridArea": true, - "gridColumn": true, - "gridColumnEnd": true, - "gridColumnStart": true, - "gridRow": true, - "gridRowEnd": true, - "gridRowStart": true, - "lineHeight": true, - "opacity": true, - "order": true, - "orphans": true, - "widows": true, - "zIndex": true, - "zoom": true - }, - // Add in properties whose names you wish to fix before // setting or getting the value cssProps: {}, @@ -267,11 +244,9 @@ jQuery.extend( { return; } - // If a number was passed in, add the unit (except for certain CSS properties) - // The isCustomProp check can be removed in jQuery 4.0 when we only auto-append - // "px" to a few hardcoded values. - if ( type === "number" && !isCustomProp ) { - value += ret && ret[ 3 ] || ( jQuery.cssNumber[ origName ] ? "" : "px" ); + // If the value is a number, add `px` for certain CSS properties + if ( type === "number" ) { + value += ret && ret[ 3 ] || ( isAutoPx( origName ) ? "px" : "" ); } // background-* props affect original clone's values diff --git a/src/css/adjustCSS.js b/src/css/adjustCSS.js index 8898789a7..4e3e6c37a 100644 --- a/src/css/adjustCSS.js +++ b/src/css/adjustCSS.js @@ -1,7 +1,8 @@ define( [ "../core", + "./isAutoPx", "../var/rcssNum" -], function( jQuery, rcssNum ) { +], function( jQuery, isAutoPx, rcssNum ) { "use strict"; @@ -16,11 +17,11 @@ function adjustCSS( elem, prop, valueParts, tween ) { return jQuery.css( elem, prop, "" ); }, initial = currentValue(), - unit = valueParts && valueParts[ 3 ] || ( jQuery.cssNumber[ prop ] ? "" : "px" ), + unit = valueParts && valueParts[ 3 ] || ( isAutoPx( prop ) ? "px" : "" ), // Starting value computation is required for potential unit mismatches initialInUnit = elem.nodeType && - ( jQuery.cssNumber[ prop ] || unit !== "px" && +initial ) && + ( !isAutoPx( prop ) || unit !== "px" && +initial ) && rcssNum.exec( jQuery.css( elem, prop ) ); if ( initialInUnit && initialInUnit[ 3 ] !== unit ) { diff --git a/src/css/isAutoPx.js b/src/css/isAutoPx.js new file mode 100644 index 000000000..ff4b7def1 --- /dev/null +++ b/src/css/isAutoPx.js @@ -0,0 +1,41 @@ +define( function() { + +"use strict"; + +var ralphaStart = /^[a-z]/, + + // The regex visualized: + // + // /----------\ + // | | /-------\ + // | / Top \ | | | + // /--- Border ---+-| Right |-+---+- Width -+---\ + // | | Bottom | | + // | \ Left / | + // | | + // | /----------\ | + // | /-------------\ | | |- END + // | | | | / Top \ | | + // | | / Margin \ | | | Right | | | + // |---------+-| |-+---+-| Bottom |-+----| + // | \ Padding / \ Left / | + // BEGIN -| | + // | /---------\ | + // | | | | + // | | / Min \ | / Width \ | + // \--------------+-| |-+---| |---/ + // \ Max / \ Height / + rautoPx = /^(?:Border(?:Top|Right|Bottom|Left)?(?:Width|)|(?:Margin|Padding)?(?:Top|Right|Bottom|Left)?|(?:Min|Max)?(?:Width|Height))$/; + +function isAutoPx( prop ) { + + // The first test is used to ensure that: + // 1. The prop starts with a lowercase letter (as we uppercase it for the second regex). + // 2. The prop is not empty. + return ralphaStart.test( prop ) && + rautoPx.test( prop[ 0 ].toUpperCase() + prop.slice( 1 ) ); +}; + +return isAutoPx; + +} ); diff --git a/src/effects/Tween.js b/src/effects/Tween.js index bf501ead0..f8c847f97 100644 --- a/src/effects/Tween.js +++ b/src/effects/Tween.js @@ -1,9 +1,10 @@ define( [ "../core", + "../css/isAutoPx", "../css/finalPropName", "../css" -], function( jQuery, finalPropName ) { +], function( jQuery, isAutoPx, finalPropName ) { "use strict"; @@ -21,7 +22,7 @@ Tween.prototype = { this.options = options; this.start = this.now = this.cur(); this.end = end; - this.unit = unit || ( jQuery.cssNumber[ prop ] ? "" : "px" ); + this.unit = unit || ( isAutoPx( prop ) ? "px" : "" ); }, cur: function() { var hooks = Tween.propHooks[ this.prop ]; diff --git a/test/unit/css.js b/test/unit/css.js index e7acd1915..20a5a77bc 100644 --- a/test/unit/css.js +++ b/test/unit/css.js @@ -1198,14 +1198,17 @@ if ( jQuery.fn.offset ) { } QUnit.test( "Do not append px (#9548, #12990, #2792)", function( assert ) { - assert.expect( 3 ); + assert.expect( 4 ); var $div = jQuery( "
" ).appendTo( "#qunit-fixture" ); $div.css( "fill-opacity", 1 ); - assert.equal( $div.css( "fill-opacity" ), 1, "Do not append px to 'fill-opacity'" ); + $div.css( "font-size", "27px" ); + $div.css( "line-height", 2 ); + assert.equal( $div.css( "line-height" ), "54px", "Do not append px to 'line-height'" ); + $div.css( "column-count", 1 ); if ( $div.css( "column-count" ) !== undefined ) { assert.equal( $div.css( "column-count" ), 1, "Do not append px to 'column-count'" ); @@ -1273,6 +1276,81 @@ QUnit[ } } ); +QUnit.test( "Do not append px to most properties not accepting integer values", function( assert ) { + assert.expect( 3 ); + + var $div = jQuery( "
" ).appendTo( "#qunit-fixture" ); + + $div.css( "font-size", "27px" ); + + $div.css( "font-size", 2 ); + assert.equal( $div.css( "font-size" ), "27px", "Do not append px to 'font-size'" ); + + $div.css( "fontSize", 2 ); + assert.equal( $div.css( "fontSize" ), "27px", "Do not append px to 'fontSize'" ); + + $div.css( "letter-spacing", "2px" ); + $div.css( "letter-spacing", 3 ); + assert.equal( $div.css( "letter-spacing" ), "2px", "Do not append px to 'letter-spacing'" ); +} ); + +QUnit.test( "Append px to whitelisted properties", function( assert ) { + var prop, + $div = jQuery( "
" ).appendTo( "#qunit-fixture" ), + whitelist = { + margin: "marginTop", + marginTop: undefined, + marginRight: undefined, + marginBottom: undefined, + marginLeft: undefined, + padding: "paddingTop", + paddingTop: undefined, + paddingRight: undefined, + paddingBottom: undefined, + paddingLeft: undefined, + top: undefined, + right: undefined, + bottom: undefined, + left: undefined, + width: undefined, + height: undefined, + minWidth: undefined, + minHeight: undefined, + maxWidth: undefined, + maxHeight: undefined, + border: "borderTopWidth", + borderWidth: "borderTopWidth", + borderTop: "borderTopWidth", + borderTopWidth: undefined, + borderRight: "borderRightWidth", + borderRightWidth: undefined, + borderBottom: "borderBottomWidth", + borderBottomWidth: undefined, + borderLeft: "borderLeftWidth", + borderLeftWidth: undefined + }; + + assert.expect( ( Object.keys( whitelist ).length ) * 2 ); + + for ( prop in whitelist ) { + var propToCheck = whitelist[ prop ] || prop, + kebabProp = prop.replace( /[A-Z]/g, function( match ) { + return "-" + match.toLowerCase(); + } ), + kebabPropToCheck = propToCheck.replace( /[A-Z]/g, function( match ) { + return "-" + match.toLowerCase(); + } ); + $div.css( prop, 3 ) + .css( "position", "absolute" ) + .css( "border-style", "solid" ); + assert.equal( $div.css( propToCheck ), "3px", "Append px to '" + prop + "'" ); + $div.css( kebabProp, 3 ) + .css( "position", "absolute" ) + .css( "border-style", "solid" ); + assert.equal( $div.css( kebabPropToCheck ), "3px", "Append px to '" + kebabProp + "'" ); + } +} ); + QUnit.test( "css('width') and css('height') should respect box-sizing, see #11004", function( assert ) { assert.expect( 4 ); -- 2.39.5