CSS: Don't automatically add "px" to properties with a few exceptions

mgol · mgol · commit 00a9c2e5f4c8 · 2019-04-08T19:26:08.000+02:00
Fixes gh-2795 Closes gh-4055 Ref gh-4009
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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( "<div>" ).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( "<div>" ).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( "<div>" ).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 );
 
