Skip to content

Commit 214e163

Browse files
committed
CSS: Correct misrepresentation of "auto" horizontal margins as 0
Fixes gh-2237 Closes gh-2276
1 parent 6e466af commit 214e163

File tree

6 files changed

+95
-37
lines changed

6 files changed

+95
-37
lines changed

src/css.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,19 @@ jQuery.cssHooks.marginRight = addGetHookIf( support.reliableMarginRight,
434434
}
435435
);
436436

437+
jQuery.cssHooks.marginLeft = addGetHookIf( support.reliableMarginLeft,
438+
function( elem, computed ) {
439+
if ( computed ) {
440+
return ( parseFloat( curCSS( elem, "marginLeft" ) ) ||
441+
elem.getBoundingClientRect().left -
442+
swap( elem, { marginLeft: 0 }, function() {
443+
return elem.getBoundingClientRect().left;
444+
} )
445+
) + "px";
446+
}
447+
}
448+
);
449+
437450
// These hooks are used by animate to expand properties
438451
jQuery.each( {
439452
margin: "",

src/css/support.js

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ define( [
66
], function( jQuery, document, documentElement, support ) {
77

88
( function() {
9-
var pixelPositionVal, boxSizingReliableVal, gBCRDimensionsVal,
10-
pixelMarginRightVal, reliableHiddenOffsetsVal, reliableMarginRightVal,
9+
var pixelPositionVal, pixelMarginRightVal, gBCRDimensionsVal, boxSizingReliableVal,
10+
reliableHiddenOffsetsVal, reliableMarginRightVal, reliableMarginLeftVal,
1111
container = document.createElement( "div" ),
1212
div = document.createElement( "div" );
1313

@@ -84,6 +84,15 @@ define( [
8484
computeStyleTests();
8585
}
8686
return reliableMarginRightVal;
87+
},
88+
89+
reliableMarginLeft: function() {
90+
91+
// Support: IE <=8 only, Android 4.0 - 4.3 only, Firefox <=3 - 37
92+
if ( pixelPositionVal == null ) {
93+
computeStyleTests();
94+
}
95+
return reliableMarginLeftVal;
8796
}
8897
} );
8998

@@ -99,14 +108,13 @@ define( [
99108
// Support: Android 2.3
100109
// Vendor-prefix box-sizing
101110
"-webkit-box-sizing:border-box;box-sizing:border-box;" +
102-
"position:absolute;display:block;" +
103-
"margin:0;margin-top:1%;margin-right:50%;" +
104-
"border:1px;padding:1px;" +
105-
"top:1%;height:4px;width:50%";
111+
"position:relative;display:block;" +
112+
"margin:auto;border:1px;padding:1px;" +
113+
"top:1%;width:50%";
106114

107115
// Support: IE<9
108116
// Assume reasonable values in the absence of getComputedStyle
109-
pixelPositionVal = boxSizingReliableVal = false;
117+
pixelPositionVal = boxSizingReliableVal = reliableMarginLeftVal = false;
110118
pixelMarginRightVal = reliableMarginRightVal = true;
111119

112120
// Support: IE<9
@@ -117,10 +125,15 @@ define( [
117125
if ( window.getComputedStyle ) {
118126
divStyle = window.getComputedStyle( div );
119127
pixelPositionVal = ( divStyle || {} ).top !== "1%";
120-
boxSizingReliableVal = ( divStyle || { height: "4px" } ).height === "4px";
128+
reliableMarginLeftVal = ( divStyle || {} ).marginLeft === "2px";
129+
boxSizingReliableVal = ( divStyle || { width: "4px" } ).width === "4px";
130+
131+
// Support: Android 4.0 - 4.3 only
132+
// Some styles come back with percentage values, even though they shouldn't
133+
div.style.marginRight = "50%";
121134
pixelMarginRightVal = ( divStyle || { marginRight: "4px" } ).marginRight === "4px";
122135

123-
// Support: Android 2.3
136+
// Support: Android 2.3 only
124137
// Div with explicit width and no margin-right incorrectly
125138
// gets computed margin-right based on width of container (#3333)
126139
// WebKit Bug 13343 - getComputedStyle returns wrong value for margin-right

test/data/offset/relative.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
body { margin: 1px; padding: 5px; }
99
div.relative { position: relative; top: 0; left: 0; margin: 1px; border: 2px solid #000; padding: 5px; width: 100px; height: 100px; background: #fff; overflow: hidden; }
1010
#relative-2 { top: 20px; left: 20px; }
11+
#relative-2-1 { margin: auto; width: 50px; }
1112
#marker { position: absolute; border: 2px solid #000; width: 50px; height: 50px; background: #ccc; }
1213
</style>
1314
<script src="../../jquery.js"></script>
@@ -24,7 +25,7 @@
2425
</head>
2526
<body>
2627
<div id="relative-1" class="relative"><div id="relative-1-1" class="relative"><div id="relative-1-1-1" class="relative"></div></div></div>
27-
<div id="relative-2" class="relative"></div>
28+
<div id="relative-2" class="relative"><div id="relative-2-1" class="relative"></div></div>
2829
<div id="marker"></div>
2930
<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>
3031
</body>

test/unit/css.js

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -754,17 +754,31 @@ QUnit.test( "internal ref to elem.runtimeStyle (bug #7608)", function( assert )
754754
assert.ok( result, "elem.runtimeStyle does not throw exception" );
755755
} );
756756

757-
QUnit.test( "marginRight computed style (bug #3333)", function( assert ) {
758-
assert.expect( 1 );
757+
QUnit.test( "computed margins (trac-3333; gh-2237)", function( assert ) {
758+
assert.expect( 2 );
759+
760+
var $div = jQuery( "#foo" ),
761+
$child = jQuery( "#en" );
759762

760-
var $div = jQuery( "#foo" );
761763
$div.css( {
762764
"width": "1px",
763765
"marginRight": 0
764766
} );
765-
766-
assert.equal( $div.css( "marginRight" ), "0px", "marginRight correctly calculated with a width and display block" );
767-
} );
767+
assert.equal( $div.css( "marginRight" ), "0px",
768+
"marginRight correctly calculated with a width and display block" );
769+
770+
$div.css({
771+
position: "absolute",
772+
top: 0,
773+
left: 0,
774+
width: "100px"
775+
});
776+
$child.css({
777+
width: "50px",
778+
margin: "auto"
779+
});
780+
assert.equal( $child.css( "marginLeft" ), "25px", "auto margins are computed to pixels" );
781+
});
768782

769783
QUnit.test( "box model properties incorrectly returning % instead of px, see #10639 and #12088", function( assert ) {
770784
assert.expect( 2 );

test/unit/offset.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,15 +187,16 @@ testIframe( "offset/absolute", "absolute", function( $, window, document, assert
187187
} );
188188

189189
testIframe( "offset/relative", "relative", function( $, window, document, assert ) {
190-
assert.expect( 60 );
190+
assert.expect( 64 );
191191

192192
var tests;
193193

194194
// get offset
195195
tests = [
196196
{ "id": "#relative-1", "top": 7, "left": 7 },
197197
{ "id": "#relative-1-1", "top": 15, "left": 15 },
198-
{ "id": "#relative-2", "top": 142, "left": 27 }
198+
{ "id": "#relative-2", "top": 142, "left": 27 },
199+
{ "id": "#relative-2-1", "top": 149, "left": 52 }
199200
];
200201
jQuery.each( tests, function() {
201202
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
206207
tests = [
207208
{ "id": "#relative-1", "top": 6, "left": 6 },
208209
{ "id": "#relative-1-1", "top": 5, "left": 5 },
209-
{ "id": "#relative-2", "top": 141, "left": 26 }
210+
{ "id": "#relative-2", "top": 141, "left": 26 },
211+
{ "id": "#relative-2-1", "top": 5, "left": 5 }
210212
];
211213
jQuery.each( tests, function() {
212214
assert.equal( $( this[ "id" ] ).position().top, this[ "top" ], "jQuery('" + this[ "id" ] + "').position().top" );

test/unit/support.js

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ testIframeWithCallback(
106106
"radioValue": true,
107107
"reliableHiddenOffsets": true,
108108
"reliableMarginRight": true,
109+
"reliableMarginLeft": true,
109110
"style": true,
110111
"submit": true
111112
};
@@ -139,6 +140,7 @@ testIframeWithCallback(
139140
"radioValue": false,
140141
"reliableHiddenOffsets": true,
141142
"reliableMarginRight": true,
143+
"reliableMarginLeft": true,
142144
"style": true,
143145
"submit": true
144146
};
@@ -172,6 +174,7 @@ testIframeWithCallback(
172174
"radioValue": false,
173175
"reliableHiddenOffsets": true,
174176
"reliableMarginRight": true,
177+
"reliableMarginLeft": true,
175178
"style": true,
176179
"submit": true
177180
};
@@ -205,6 +208,7 @@ testIframeWithCallback(
205208
"radioValue": false,
206209
"reliableHiddenOffsets": false,
207210
"reliableMarginRight": true,
211+
"reliableMarginLeft": false,
208212
"style": false,
209213
"submit": false
210214
};
@@ -241,6 +245,7 @@ testIframeWithCallback(
241245
"radioValue": true,
242246
"reliableHiddenOffsets": true,
243247
"reliableMarginRight": true,
248+
"reliableMarginLeft": true,
244249
"style": true,
245250
"submit": true
246251
};
@@ -274,6 +279,7 @@ testIframeWithCallback(
274279
"radioValue": true,
275280
"reliableHiddenOffsets": true,
276281
"reliableMarginRight": true,
282+
"reliableMarginLeft": true,
277283
"style": true,
278284
"submit": true
279285
};
@@ -307,6 +313,7 @@ testIframeWithCallback(
307313
"radioValue": true,
308314
"reliableHiddenOffsets": true,
309315
"reliableMarginRight": true,
316+
"reliableMarginLeft": true,
310317
"style": true,
311318
"submit": true
312319
};
@@ -340,6 +347,7 @@ testIframeWithCallback(
340347
"radioValue": true,
341348
"reliableHiddenOffsets": true,
342349
"reliableMarginRight": true,
350+
"reliableMarginLeft": false,
343351
"style": true,
344352
"submit": true
345353
};
@@ -373,6 +381,7 @@ testIframeWithCallback(
373381
"radioValue": true,
374382
"reliableHiddenOffsets": true,
375383
"reliableMarginRight": true,
384+
"reliableMarginLeft": true,
376385
"style": true,
377386
"submit": true
378387
};
@@ -406,6 +415,7 @@ testIframeWithCallback(
406415
"radioValue": true,
407416
"reliableHiddenOffsets": true,
408417
"reliableMarginRight": true,
418+
"reliableMarginLeft": true,
409419
"style": true,
410420
"submit": true
411421
};
@@ -439,6 +449,7 @@ testIframeWithCallback(
439449
"radioValue": true,
440450
"reliableHiddenOffsets": true,
441451
"reliableMarginRight": true,
452+
"reliableMarginLeft": false,
442453
"style": true,
443454
"submit": true
444455
};
@@ -472,32 +483,36 @@ testIframeWithCallback(
472483
"radioValue": true,
473484
"reliableHiddenOffsets": true,
474485
"reliableMarginRight": false,
486+
"reliableMarginLeft": true,
475487
"style": true,
476488
"submit": true
477489
};
478490
}
479491

480-
if ( expected ) {
481-
QUnit.test( "Verify that the support tests resolve as expected per browser", function( assert ) {
482-
var i, prop,
483-
j = 0;
492+
QUnit.test( "Verify that support tests resolve as expected per browser", function( assert ) {
493+
if ( !expected ) {
494+
assert.expect( 1 );
495+
assert.ok( false, "Known client: " + userAgent );
496+
}
484497

485-
for ( prop in computedSupport ) {
486-
j++;
487-
}
498+
var i, prop,
499+
j = 0;
500+
501+
for ( prop in computedSupport ) {
502+
j++;
503+
}
488504

489-
assert.expect( j );
505+
assert.expect( j );
490506

491-
for ( i in expected ) {
492-
if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) {
493-
assert.equal( computedSupport[ i ], expected[ i ],
494-
"jQuery.support['" + i + "']: " + computedSupport[ i ] +
495-
", expected['" + i + "']: " + expected[ i ] );
496-
} else {
497-
assert.ok( true, "no ajax; skipping jQuery.support['" + i + "']" );
498-
}
507+
for ( i in expected ) {
508+
if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) {
509+
assert.equal( computedSupport[ i ], expected[ i ],
510+
"jQuery.support['" + i + "']: " + computedSupport[ i ] +
511+
", expected['" + i + "']: " + expected[ i ] );
512+
} else {
513+
assert.ok( true, "no ajax; skipping jQuery.support['" + i + "']" );
499514
}
500-
} );
501-
}
515+
}
516+
});
502517

503518
} )();

0 commit comments

Comments
 (0)