From 214e1634ab9b1d13d53647dd5de3bdf7a091d49c 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 --- src/css.js | 13 +++++++++ src/css/support.js | 31 +++++++++++++++------ test/data/offset/relative.html | 3 +- test/unit/css.js | 26 +++++++++++++---- test/unit/offset.js | 8 ++++-- test/unit/support.js | 51 ++++++++++++++++++++++------------ 6 files changed, 95 insertions(+), 37 deletions(-) diff --git a/src/css.js b/src/css.js index 17c47453a..2a82b3845 100644 --- a/src/css.js +++ b/src/css.js @@ -434,6 +434,19 @@ jQuery.cssHooks.marginRight = addGetHookIf( support.reliableMarginRight, } ); +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 96276a9e5..549cfa680 100644 --- a/src/css/support.js +++ b/src/css/support.js @@ -6,8 +6,8 @@ define( [ ], function( jQuery, document, documentElement, support ) { ( function() { - var pixelPositionVal, boxSizingReliableVal, gBCRDimensionsVal, - pixelMarginRightVal, reliableHiddenOffsetsVal, reliableMarginRightVal, + var pixelPositionVal, pixelMarginRightVal, gBCRDimensionsVal, boxSizingReliableVal, + reliableHiddenOffsetsVal, reliableMarginRightVal, reliableMarginLeftVal, container = document.createElement( "div" ), div = document.createElement( "div" ); @@ -84,6 +84,15 @@ define( [ computeStyleTests(); } return reliableMarginRightVal; + }, + + reliableMarginLeft: function() { + + // Support: IE <=8 only, Android 4.0 - 4.3 only, Firefox <=3 - 37 + if ( pixelPositionVal == null ) { + computeStyleTests(); + } + return reliableMarginLeftVal; } } ); @@ -99,14 +108,13 @@ define( [ // Support: Android 2.3 // Vendor-prefix box-sizing "-webkit-box-sizing:border-box;box-sizing:border-box;" + - "position:absolute;display:block;" + - "margin:0;margin-top:1%;margin-right:50%;" + - "border:1px;padding:1px;" + - "top:1%;height:4px;width:50%"; + "position:relative;display:block;" + + "margin:auto;border:1px;padding:1px;" + + "top:1%;width:50%"; // Support: IE<9 // Assume reasonable values in the absence of getComputedStyle - pixelPositionVal = boxSizingReliableVal = false; + pixelPositionVal = boxSizingReliableVal = reliableMarginLeftVal = false; pixelMarginRightVal = reliableMarginRightVal = true; // Support: IE<9 @@ -117,10 +125,15 @@ define( [ if ( window.getComputedStyle ) { divStyle = window.getComputedStyle( div ); pixelPositionVal = ( divStyle || {} ).top !== "1%"; - boxSizingReliableVal = ( divStyle || { height: "4px" } ).height === "4px"; + reliableMarginLeftVal = ( divStyle || {} ).marginLeft === "2px"; + boxSizingReliableVal = ( divStyle || { width: "4px" } ).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" } ).marginRight === "4px"; - // Support: Android 2.3 + // Support: Android 2.3 only // Div with explicit width and no margin-right incorrectly // gets computed margin-right based on width of container (#3333) // WebKit Bug 13343 - getComputedStyle returns wrong value for margin-right diff --git a/test/data/offset/relative.html b/test/data/offset/relative.html index 3ac054837..317006994 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 6b031f61f..63a49f848 100644 --- a/test/unit/css.js +++ b/test/unit/css.js @@ -754,17 +754,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 11633b6a7..76b2e897e 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -187,7 +187,7 @@ testIframe( "offset/absolute", "absolute", function( $, window, document, assert } ); testIframe( "offset/relative", "relative", function( $, window, document, assert ) { - assert.expect( 60 ); + assert.expect( 64 ); var tests; @@ -195,7 +195,8 @@ testIframe( "offset/relative", "relative", function( $, window, document, assert 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" ); @@ -206,7 +207,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 b5c1594c0..4818b489e 100644 --- a/test/unit/support.js +++ b/test/unit/support.js @@ -106,6 +106,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -139,6 +140,7 @@ testIframeWithCallback( "radioValue": false, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -172,6 +174,7 @@ testIframeWithCallback( "radioValue": false, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -205,6 +208,7 @@ testIframeWithCallback( "radioValue": false, "reliableHiddenOffsets": false, "reliableMarginRight": true, + "reliableMarginLeft": false, "style": false, "submit": false }; @@ -241,6 +245,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -274,6 +279,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -307,6 +313,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -340,6 +347,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": false, "style": true, "submit": true }; @@ -373,6 +381,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -406,6 +415,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": true, "style": true, "submit": true }; @@ -439,6 +449,7 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": true, + "reliableMarginLeft": false, "style": true, "submit": true }; @@ -472,32 +483,36 @@ testIframeWithCallback( "radioValue": true, "reliableHiddenOffsets": true, "reliableMarginRight": false, + "reliableMarginLeft": true, "style": true, "submit": true }; } - 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; + + for ( prop in computedSupport ) { + j++; + } - assert.expect( j ); + assert.expect( j ); - 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 + "']" ); - } + 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