From 487d5ca913c237aafe9efa1179749b46382fddbf Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 7 May 2015 23:16:18 -0400 Subject: [PATCH] CSS: Correct misrepresentation of "auto" horizontal margins as 0 Fixes gh-2237 Closes gh-2276 (cherry picked from commit 214e1634ab9b1d13d53647dd5de3bdf7a091d49c) Conflicts: src/css.js src/css/support.js test/unit/support.js --- src/css.js | 13 +++++++ src/css/support.js | 24 +++++++++--- test/data/offset/relative.html | 3 +- test/unit/css.js | 26 ++++++++++--- test/unit/offset.js | 8 ++-- test/unit/support.js | 69 ++++++++++++++++++++-------------- 6 files changed, 98 insertions(+), 45 deletions(-) diff --git a/src/css.js b/src/css.js index 4fbc6745e..0e5a55610 100644 --- a/src/css.js +++ b/src/css.js @@ -350,6 +350,19 @@ jQuery.each( [ "height", "width" ], function( i, name ) { }; } ); +jQuery.cssHooks.marginLeft = addGetHookIf( support.reliableMarginLeft, + function( elem, computed ) { + if ( computed ) { + return ( parseFloat( curCSS( elem, "marginLeft" ) ) || + elem.getBoundingClientRect().left - + swap( elem, { marginLeft: 0 }, function() { + return elem.getBoundingClientRect().left; + } ) + ) + "px"; + } + } +); + // These hooks are used by animate to expand properties jQuery.each( { margin: "", diff --git a/src/css/support.js b/src/css/support.js index 08560b42a..f8e02d048 100644 --- a/src/css/support.js +++ b/src/css/support.js @@ -6,7 +6,7 @@ define( [ ], function( jQuery, document, documentElement, support ) { ( function() { - var pixelPositionVal, boxSizingReliableVal, pixelMarginRightVal, + var pixelPositionVal, boxSizingReliableVal, pixelMarginRightVal, reliableMarginLeftVal, container = document.createElement( "div" ), div = document.createElement( "div" ); @@ -30,16 +30,20 @@ define( [ function computeStyleTests() { div.style.cssText = "box-sizing:border-box;" + - "display:block;position:absolute;" + - "margin:0;margin-top:1%;margin-right:50%;" + - "border:1px;padding:1px;" + - "top:1%;width:50%;height:4px"; + "position:relative;display:block;" + + "margin:auto;border:1px;padding:1px;" + + "top:1%;width:50%"; div.innerHTML = ""; documentElement.appendChild( container ); var divStyle = window.getComputedStyle( div ); pixelPositionVal = divStyle.top !== "1%"; - boxSizingReliableVal = divStyle.height === "4px"; + reliableMarginLeftVal = divStyle.marginLeft === "2px"; + boxSizingReliableVal = divStyle.width === "4px"; + + // Support: Android 4.0 - 4.3 only + // Some styles come back with percentage values, even though they shouldn't + div.style.marginRight = "50%"; pixelMarginRightVal = divStyle.marginRight === "4px"; documentElement.removeChild( container ); @@ -69,6 +73,14 @@ define( [ computeStyleTests(); } return pixelMarginRightVal; + }, + reliableMarginLeft: function() { + + // Support: IE <=8 only, Android 4.0 - 4.3 only, Firefox <=3 - 37 + if ( boxSizingReliableVal == null ) { + computeStyleTests(); + } + return reliableMarginLeftVal; } } ); } )(); diff --git a/test/data/offset/relative.html b/test/data/offset/relative.html index e81186625..f88c82ab9 100644 --- a/test/data/offset/relative.html +++ b/test/data/offset/relative.html @@ -8,6 +8,7 @@ body { margin: 1px; padding: 5px; } div.relative { position: relative; top: 0; left: 0; margin: 1px; border: 2px solid #000; padding: 5px; width: 100px; height: 100px; background: #fff; overflow: hidden; } #relative-2 { top: 20px; left: 20px; } + #relative-2-1 { margin: auto; width: 50px; } #marker { position: absolute; border: 2px solid #000; width: 50px; height: 50px; background: #ccc; } @@ -24,7 +25,7 @@

Click the white box to move the marker to it. Clicking the box also changes the position to absolute (if not already) and sets the position according to the position method.

diff --git a/test/unit/css.js b/test/unit/css.js index 5c9f3e01f..343459dfb 100644 --- a/test/unit/css.js +++ b/test/unit/css.js @@ -728,17 +728,31 @@ QUnit.test( "internal ref to elem.runtimeStyle (bug #7608)", function( assert ) assert.ok( result, "elem.runtimeStyle does not throw exception" ); } ); -QUnit.test( "marginRight computed style (bug #3333)", function( assert ) { - assert.expect( 1 ); +QUnit.test( "computed margins (trac-3333; gh-2237)", function( assert ) { + assert.expect( 2 ); + + var $div = jQuery( "#foo" ), + $child = jQuery( "#en" ); - var $div = jQuery( "#foo" ); $div.css( { "width": "1px", "marginRight": 0 } ); - - assert.equal( $div.css( "marginRight" ), "0px", "marginRight correctly calculated with a width and display block" ); -} ); + assert.equal( $div.css( "marginRight" ), "0px", + "marginRight correctly calculated with a width and display block" ); + + $div.css({ + position: "absolute", + top: 0, + left: 0, + width: "100px" + }); + $child.css({ + width: "50px", + margin: "auto" + }); + assert.equal( $child.css( "marginLeft" ), "25px", "auto margins are computed to pixels" ); +}); QUnit.test( "box model properties incorrectly returning % instead of px, see #10639 and #12088", function( assert ) { assert.expect( 2 ); diff --git a/test/unit/offset.js b/test/unit/offset.js index f0cd47769..85f1da65d 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -186,13 +186,14 @@ testIframe( "offset/absolute", "absolute", function( $, window, document, assert } ); testIframe( "offset/relative", "relative", function( $, window, document, assert ) { - assert.expect( 60 ); + assert.expect( 64 ); // get offset var tests = [ { "id": "#relative-1", "top": 7, "left": 7 }, { "id": "#relative-1-1", "top": 15, "left": 15 }, - { "id": "#relative-2", "top": 142, "left": 27 } + { "id": "#relative-2", "top": 142, "left": 27 }, + { "id": "#relative-2-1", "top": 149, "left": 52 } ]; jQuery.each( tests, function() { assert.equal( $( this[ "id" ] ).offset().top, this[ "top" ], "jQuery('" + this[ "id" ] + "').offset().top" ); @@ -203,7 +204,8 @@ testIframe( "offset/relative", "relative", function( $, window, document, assert tests = [ { "id": "#relative-1", "top": 6, "left": 6 }, { "id": "#relative-1-1", "top": 5, "left": 5 }, - { "id": "#relative-2", "top": 141, "left": 26 } + { "id": "#relative-2", "top": 141, "left": 26 }, + { "id": "#relative-2-1", "top": 5, "left": 5 } ]; jQuery.each( tests, function() { assert.equal( $( this[ "id" ] ).position().top, this[ "top" ], "jQuery('" + this[ "id" ] + "').position().top" ); diff --git a/test/unit/support.js b/test/unit/support.js index 8c6b0a28b..a62e57c73 100644 --- a/test/unit/support.js +++ b/test/unit/support.js @@ -70,7 +70,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": true, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /(msie 10\.0|trident\/7\.0)/i.test( userAgent ) ) { expected = { @@ -86,7 +87,8 @@ testIframeWithCallback( "optSelected": false, "pixelMarginRight": true, "pixelPosition": true, - "radioValue": false + "radioValue": false, + "reliableMarginLeft": true }; } else if ( /msie 9\.0/i.test( userAgent ) ) { expected = { @@ -102,7 +104,8 @@ testIframeWithCallback( "optSelected": false, "pixelMarginRight": true, "pixelPosition": true, - "radioValue": false + "radioValue": false, + "reliableMarginLeft": true }; } else if ( /chrome/i.test( userAgent ) ) { @@ -121,7 +124,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": true, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /8\.0(\.\d+|) safari/i.test( userAgent ) ) { expected = { @@ -137,7 +141,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": false, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /7\.0(\.\d+|) safari/i.test( userAgent ) ) { expected = { @@ -153,7 +158,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": false, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /firefox/i.test( userAgent ) ) { expected = { @@ -169,7 +175,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": true, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": false }; } else if ( /iphone os 8/i.test( userAgent ) ) { expected = { @@ -185,7 +192,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": false, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /iphone os (6|7)/i.test( userAgent ) ) { expected = { @@ -201,7 +209,8 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": true, "pixelPosition": false, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": true }; } else if ( /android 4\.[0-3]/i.test( userAgent ) ) { expected = { @@ -217,33 +226,35 @@ testIframeWithCallback( "optSelected": true, "pixelMarginRight": false, "pixelPosition": false, - "radioValue": true + "radioValue": true, + "reliableMarginLeft": false }; } - if ( expected ) { - QUnit.test( "Verify that the support tests resolve as expected per browser", function( assert ) { - var i, prop, - j = 0; + QUnit.test( "Verify that support tests resolve as expected per browser", function( assert ) { + if ( !expected ) { + assert.expect( 1 ); + assert.ok( false, "Known client: " + userAgent ); + } - for ( prop in computedSupport ) { - j++; - } + var i, prop, + j = 0; - assert.expect( j ); + for ( prop in computedSupport ) { + j++; + } - for ( i in expected ) { + assert.expect( j ); - // TODO check for all modules containing support properties - if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) { - assert.equal( computedSupport[ i ], expected[ i ], - "jQuery.support['" + i + "']: " + computedSupport[ i ] + - ", expected['" + i + "']: " + expected[ i ] ); - } else { - assert.ok( true, "no ajax; skipping jQuery.support[' " + i + " ']" ); - } + for ( i in expected ) { + if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) { + assert.equal( computedSupport[ i ], expected[ i ], + "jQuery.support['" + i + "']: " + computedSupport[ i ] + + ", expected['" + i + "']: " + expected[ i ] ); + } else { + assert.ok( true, "no ajax; skipping jQuery.support['" + i + "']" ); } - } ); - } + } + }); } )(); -- 2.39.5