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

Fixes gh-2795
Closes gh-4055
Ref gh-4009

diff --git a/src/css.js b/src/css.js
index ac4c66e87acf74ac164567aae75f617d2f94d4e1..d6354a3103d75192ded85439e53a710b08a5845a 100644 (file)
--- 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 8898789a7bd97eea0c175a3aaefbc13cd4957cb7..4e3e6c37a60f1213072e7edbdd7a22bd37b48ce8 100644 (file)
--- 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 (file)
index 0000000..ff4b7de
--- /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 bf501ead074b8a2bf67738eb97c681028625a2a5..f8c847f97232f71597638b2acb62cf6e1050653c 100644 (file)
--- 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 e7acd191506ab268bc08369490c4ec9d886b4a5d..20a5a77bc4e945bc55cfe4aa1e18f37359014987 100644 (file)
--- 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 );