Skip to content

Commit

Permalink
CSS: Correct misrepresentation of "auto" horizontal margins as 0
Browse files Browse the repository at this point in the history
Fixes gh-2237
Closes gh-2276

(cherry picked from commit 214e163)

Conflicts:
	src/css.js
	src/css/support.js
	test/unit/support.js
  • Loading branch information
gibson042 committed Oct 18, 2015
1 parent c752a50 commit 487d5ca
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 45 deletions.
13 changes: 13 additions & 0 deletions src/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down
24 changes: 18 additions & 6 deletions src/css/support.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" );

Expand All @@ -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 );
Expand Down Expand Up @@ -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;
}
} );
} )();
Expand Down
3 changes: 2 additions & 1 deletion test/data/offset/relative.html
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
</style>
<script src="../../jquery.js"></script>
Expand All @@ -24,7 +25,7 @@
</head>
<body>
<div id="relative-1" class="relative"><div id="relative-1-1" class="relative"><div id="relative-1-1-1" class="relative"></div></div></div>
<div id="relative-2" class="relative"></div>
<div id="relative-2" class="relative"><div id="relative-2-1" class="relative"></div></div>
<div id="marker"></div>
<p class="instructions">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.</p>
</body>
Expand Down
26 changes: 20 additions & 6 deletions test/unit/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
8 changes: 5 additions & 3 deletions test/unit/offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" );
Expand All @@ -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" );
Expand Down
69 changes: 40 additions & 29 deletions test/unit/support.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -102,7 +104,8 @@ testIframeWithCallback(
"optSelected": false,
"pixelMarginRight": true,
"pixelPosition": true,
"radioValue": false
"radioValue": false,
"reliableMarginLeft": true
};
} else if ( /chrome/i.test( userAgent ) ) {

Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -153,7 +158,8 @@ testIframeWithCallback(
"optSelected": true,
"pixelMarginRight": true,
"pixelPosition": false,
"radioValue": true
"radioValue": true,
"reliableMarginLeft": true
};
} else if ( /firefox/i.test( userAgent ) ) {
expected = {
Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -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 + "']" );
}
} );
}
}
});

} )();

0 comments on commit 487d5ca

Please sign in to comment.