Skip to content

Commit 487d5ca

Browse files
gibson042gibson042
committed
CSS: Correct misrepresentation of "auto" horizontal margins as 0
Fixes gh-2237 Closes gh-2276 (cherry picked from commit 214e163) Conflicts: src/css.js src/css/support.js test/unit/support.js
1 parent c752a50 commit 487d5ca

File tree

6 files changed

+98
-45
lines changed

6 files changed

+98
-45
lines changed

‎src/css.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,19 @@ jQuery.each( [ "height", "width" ], function( i, name ) {
350350
};
351351
} );
352352

353+
jQuery.cssHooks.marginLeft = addGetHookIf( support.reliableMarginLeft,
354+
function( elem, computed ) {
355+
if ( computed ) {
356+
return ( parseFloat( curCSS( elem, "marginLeft" ) ) ||
357+
elem.getBoundingClientRect().left -
358+
swap( elem, { marginLeft: 0 }, function() {
359+
return elem.getBoundingClientRect().left;
360+
} )
361+
) + "px";
362+
}
363+
}
364+
);
365+
353366
// These hooks are used by animate to expand properties
354367
jQuery.each( {
355368
margin: "",

‎src/css/support.js‎

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

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

@@ -30,16 +30,20 @@ define( [
3030
function computeStyleTests() {
3131
div.style.cssText =
3232
"box-sizing:border-box;" +
33-
"display:block;position:absolute;" +
34-
"margin:0;margin-top:1%;margin-right:50%;" +
35-
"border:1px;padding:1px;" +
36-
"top:1%;width:50%;height:4px";
33+
"position:relative;display:block;" +
34+
"margin:auto;border:1px;padding:1px;" +
35+
"top:1%;width:50%";
3736
div.innerHTML = "";
3837
documentElement.appendChild( container );
3938

4039
var divStyle = window.getComputedStyle( div );
4140
pixelPositionVal = divStyle.top !== "1%";
42-
boxSizingReliableVal = divStyle.height === "4px";
41+
reliableMarginLeftVal = divStyle.marginLeft === "2px";
42+
boxSizingReliableVal = divStyle.width === "4px";
43+
44+
// Support: Android 4.0 - 4.3 only
45+
// Some styles come back with percentage values, even though they shouldn't
46+
div.style.marginRight = "50%";
4347
pixelMarginRightVal = divStyle.marginRight === "4px";
4448

4549
documentElement.removeChild( container );
@@ -69,6 +73,14 @@ define( [
6973
computeStyleTests();
7074
}
7175
return pixelMarginRightVal;
76+
},
77+
reliableMarginLeft: function() {
78+
79+
// Support: IE <=8 only, Android 4.0 - 4.3 only, Firefox <=3 - 37
80+
if ( boxSizingReliableVal == null ) {
81+
computeStyleTests();
82+
}
83+
return reliableMarginLeftVal;
7284
}
7385
} );
7486
} )();

‎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
@@ -728,17 +728,31 @@ QUnit.test( "internal ref to elem.runtimeStyle (bug #7608)", function( assert )
728728
assert.ok( result, "elem.runtimeStyle does not throw exception" );
729729
} );
730730

731-
QUnit.test( "marginRight computed style (bug #3333)", function( assert ) {
732-
assert.expect( 1 );
731+
QUnit.test( "computed margins (trac-3333; gh-2237)", function( assert ) {
732+
assert.expect( 2 );
733+
734+
var $div = jQuery( "#foo" ),
735+
$child = jQuery( "#en" );
733736

734-
var $div = jQuery( "#foo" );
735737
$div.css( {
736738
"width": "1px",
737739
"marginRight": 0
738740
} );
739-
740-
assert.equal( $div.css( "marginRight" ), "0px", "marginRight correctly calculated with a width and display block" );
741-
} );
741+
assert.equal( $div.css( "marginRight" ), "0px",
742+
"marginRight correctly calculated with a width and display block" );
743+
744+
$div.css({
745+
position: "absolute",
746+
top: 0,
747+
left: 0,
748+
width: "100px"
749+
});
750+
$child.css({
751+
width: "50px",
752+
margin: "auto"
753+
});
754+
assert.equal( $child.css( "marginLeft" ), "25px", "auto margins are computed to pixels" );
755+
});
742756

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

‎test/unit/offset.js‎

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

188188
testIframe( "offset/relative", "relative", function( $, window, document, assert ) {
189-
assert.expect( 60 );
189+
assert.expect( 64 );
190190

191191
// get offset
192192
var tests = [
193193
{ "id": "#relative-1", "top": 7, "left": 7 },
194194
{ "id": "#relative-1-1", "top": 15, "left": 15 },
195-
{ "id": "#relative-2", "top": 142, "left": 27 }
195+
{ "id": "#relative-2", "top": 142, "left": 27 },
196+
{ "id": "#relative-2-1", "top": 149, "left": 52 }
196197
];
197198
jQuery.each( tests, function() {
198199
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
203204
tests = [
204205
{ "id": "#relative-1", "top": 6, "left": 6 },
205206
{ "id": "#relative-1-1", "top": 5, "left": 5 },
206-
{ "id": "#relative-2", "top": 141, "left": 26 }
207+
{ "id": "#relative-2", "top": 141, "left": 26 },
208+
{ "id": "#relative-2-1", "top": 5, "left": 5 }
207209
];
208210
jQuery.each( tests, function() {
209211
assert.equal( $( this[ "id" ] ).position().top, this[ "top" ], "jQuery('" + this[ "id" ] + "').position().top" );

‎test/unit/support.js‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ testIframeWithCallback(
7070
"optSelected": true,
7171
"pixelMarginRight": true,
7272
"pixelPosition": true,
73-
"radioValue": true
73+
"radioValue": true,
74+
"reliableMarginLeft": true
7475
};
7576
} else if ( /(msie 10\.0|trident\/7\.0)/i.test( userAgent ) ) {
7677
expected = {
@@ -86,7 +87,8 @@ testIframeWithCallback(
8687
"optSelected": false,
8788
"pixelMarginRight": true,
8889
"pixelPosition": true,
89-
"radioValue": false
90+
"radioValue": false,
91+
"reliableMarginLeft": true
9092
};
9193
} else if ( /msie 9\.0/i.test( userAgent ) ) {
9294
expected = {
@@ -102,7 +104,8 @@ testIframeWithCallback(
102104
"optSelected": false,
103105
"pixelMarginRight": true,
104106
"pixelPosition": true,
105-
"radioValue": false
107+
"radioValue": false,
108+
"reliableMarginLeft": true
106109
};
107110
} else if ( /chrome/i.test( userAgent ) ) {
108111

@@ -121,7 +124,8 @@ testIframeWithCallback(
121124
"optSelected": true,
122125
"pixelMarginRight": true,
123126
"pixelPosition": true,
124-
"radioValue": true
127+
"radioValue": true,
128+
"reliableMarginLeft": true
125129
};
126130
} else if ( /8\.0(\.\d+|) safari/i.test( userAgent ) ) {
127131
expected = {
@@ -137,7 +141,8 @@ testIframeWithCallback(
137141
"optSelected": true,
138142
"pixelMarginRight": true,
139143
"pixelPosition": false,
140-
"radioValue": true
144+
"radioValue": true,
145+
"reliableMarginLeft": true
141146
};
142147
} else if ( /7\.0(\.\d+|) safari/i.test( userAgent ) ) {
143148
expected = {
@@ -153,7 +158,8 @@ testIframeWithCallback(
153158
"optSelected": true,
154159
"pixelMarginRight": true,
155160
"pixelPosition": false,
156-
"radioValue": true
161+
"radioValue": true,
162+
"reliableMarginLeft": true
157163
};
158164
} else if ( /firefox/i.test( userAgent ) ) {
159165
expected = {
@@ -169,7 +175,8 @@ testIframeWithCallback(
169175
"optSelected": true,
170176
"pixelMarginRight": true,
171177
"pixelPosition": true,
172-
"radioValue": true
178+
"radioValue": true,
179+
"reliableMarginLeft": false
173180
};
174181
} else if ( /iphone os 8/i.test( userAgent ) ) {
175182
expected = {
@@ -185,7 +192,8 @@ testIframeWithCallback(
185192
"optSelected": true,
186193
"pixelMarginRight": true,
187194
"pixelPosition": false,
188-
"radioValue": true
195+
"radioValue": true,
196+
"reliableMarginLeft": true
189197
};
190198
} else if ( /iphone os (6|7)/i.test( userAgent ) ) {
191199
expected = {
@@ -201,7 +209,8 @@ testIframeWithCallback(
201209
"optSelected": true,
202210
"pixelMarginRight": true,
203211
"pixelPosition": false,
204-
"radioValue": true
212+
"radioValue": true,
213+
"reliableMarginLeft": true
205214
};
206215
} else if ( /android 4\.[0-3]/i.test( userAgent ) ) {
207216
expected = {
@@ -217,33 +226,35 @@ testIframeWithCallback(
217226
"optSelected": true,
218227
"pixelMarginRight": false,
219228
"pixelPosition": false,
220-
"radioValue": true
229+
"radioValue": true,
230+
"reliableMarginLeft": false
221231
};
222232
}
223233

224-
if ( expected ) {
225-
QUnit.test( "Verify that the support tests resolve as expected per browser", function( assert ) {
226-
var i, prop,
227-
j = 0;
234+
QUnit.test( "Verify that support tests resolve as expected per browser", function( assert ) {
235+
if ( !expected ) {
236+
assert.expect( 1 );
237+
assert.ok( false, "Known client: " + userAgent );
238+
}
228239

229-
for ( prop in computedSupport ) {
230-
j++;
231-
}
240+
var i, prop,
241+
j = 0;
232242

233-
assert.expect( j );
243+
for ( prop in computedSupport ) {
244+
j++;
245+
}
234246

235-
for ( i in expected ) {
247+
assert.expect( j );
236248

237-
// TODO check for all modules containing support properties
238-
if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) {
239-
assert.equal( computedSupport[ i ], expected[ i ],
240-
"jQuery.support['" + i + "']: " + computedSupport[ i ] +
241-
", expected['" + i + "']: " + expected[ i ] );
242-
} else {
243-
assert.ok( true, "no ajax; skipping jQuery.support[' " + i + " ']" );
244-
}
249+
for ( i in expected ) {
250+
if ( jQuery.ajax || i !== "ajax" && i !== "cors" ) {
251+
assert.equal( computedSupport[ i ], expected[ i ],
252+
"jQuery.support['" + i + "']: " + computedSupport[ i ] +
253+
", expected['" + i + "']: " + expected[ i ] );
254+
} else {
255+
assert.ok( true, "no ajax; skipping jQuery.support['" + i + "']" );
245256
}
246-
} );
247-
}
257+
}
258+
});
248259

249260
} )();

0 commit comments

Comments
 (0)