Diff
Modified: trunk/LayoutTests/ChangeLog (120858 => 120859)
--- trunk/LayoutTests/ChangeLog 2012-06-20 20:48:07 UTC (rev 120858)
+++ trunk/LayoutTests/ChangeLog 2012-06-20 21:04:16 UTC (rev 120859)
@@ -1,3 +1,32 @@
+2012-06-20 Julien Chaffraix <[email protected]>
+
+ REGRESSION(r113885): Margin not properly applied to elements with align=center
+ https://bugs.webkit.org/show_bug.cgi?id=89515
+
+ Reviewed by Levi Weintraub.
+
+ Reviewed by Levi Weintraub.
+
+ * fast/block/negative-margin-start-positive-margin-end-expected.html: Added.
+ * fast/block/negative-margin-start-positive-margin-end.html: Added.
+ * fast/block/negative-start-margin-align-center-expected.html: Added.
+ * fast/block/negative-start-margin-align-center.html: Added.
+ * fast/block/positive-margin-block-child-align-center-expected.html: Added.
+ * fast/block/positive-margin-block-child-align-center.html: Added.
+ * fast/block/positive-margin-start-align-center-expected.html: Added.
+ * fast/block/positive-margin-start-align-center.html: Added.
+ * fast/block/positive-margin-start-negative-margin-end-align-center-expected.html: Added.
+ * fast/block/positive-margin-start-negative-margin-end-align-center.html: Added.
+ Those checks the combination of margin start / end both positive and negative.
+
+ * fast/block/positive-margin-block-child-align-center-rtl-expected.html: Added.
+ * fast/block/positive-margin-block-child-align-center-rtl.html: Added.
+ One ltr test as I didn't find any.
+
+ * fast/table/table-cell-negative-start-margin-align-center-expected.html: Added.
+ * fast/table/table-cell-negative-start-margin-align-center.html: Added.
+ This test is very similar to the one above but involves a table.
+
2012-06-20 Robert Hogan <[email protected]>
r120844: Skip two tests on Qt
Added: trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 150px;
+ position: absolute;
+ left: 50px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end.html (0 => 120859)
--- trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end.html (rev 0)
+++ trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 150px;
+ position: absolute;
+ left: 50px;
+ background-color: green;
+}
+
+.sized
+{
+ margin-left: 100px;
+ width: 200px;
+}
+
+.marginLeft
+{
+ font: Ahem 10px;
+ height: 100px;
+ margin-left: -50px;
+ margin-right: 100px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft">xxxxxxxxxx xxxxxxxxxx xxxxxxxxxx</div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/negative-margin-start-positive-margin-end.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/negative-start-margin-align-center-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/negative-start-margin-align-center-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/negative-start-margin-align-center-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 250px;
+ position: absolute;
+ left: 50px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/negative-start-margin-align-center-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/negative-start-margin-align-center.html (0 => 120859)
--- trunk/LayoutTests/fast/block/negative-start-margin-align-center.html (rev 0)
+++ trunk/LayoutTests/fast/block/negative-start-margin-align-center.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 250px;
+ position: absolute;
+ left: 50px;
+ background-color: green;
+}
+
+.sized
+{
+ margin-left: 100px;
+ width: 200px;
+}
+
+.marginLeft
+{
+ font: Ahem 10px;
+ height: 100px;
+ margin-left: -50px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft">xxxxx xxxxx xxxxx xxxxx</div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/negative-start-margin-align-center.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget {
+ position: absolute;
+ left: 100px;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget {
+ position: absolute;
+ left: 100px;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+div {
+ height: 100px;
+}
+
+.hidTarget {
+ position: absolute;
+ left: 100px;
+ width: 100px;
+ background-color: green;
+}
+
+.sized
+{
+ width: 200px;
+ margin-left: 100px;
+ -webkit-direction: rtl;
+ direction: rtl;
+}
+
+.marginLeft
+{
+ margin-right: 100px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft"></div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-rtl.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+div {
+ height: 100px;
+}
+
+.hidTarget {
+ position: absolute;
+ left: 100px;
+ width: 100px;
+ background-color: green;
+}
+
+.sized
+{
+ width: 200px;
+}
+
+.marginLeft
+{
+ margin-left: 100px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft"></div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-start-align-center-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-start-align-center-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-start-align-center-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 100px;
+ position: absolute;
+ left: 200px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-start-align-center-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-start-align-center.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-start-align-center.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-start-align-center.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 100px;
+ position: absolute;
+ left: 200px;
+ background-color: green;
+}
+
+.sized
+{
+ width: 200px;
+}
+
+.marginLeft
+{
+ width: 100px;
+ height: 100px;
+ margin-left: 200px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft"></div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-start-align-center.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center-expected.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body
+{
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 150px;
+ position: absolute;
+ left: 200px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center-expected.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center.html (0 => 120859)
--- trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center.html (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+ margin: 0px;
+}
+
+.hidTarget
+{
+ height: 100px;
+ width: 150px;
+ position: absolute;
+ left: 200px;
+ background-color: green;
+}
+
+.sized
+{
+ margin-left: 100px;
+ width: 200px;
+}
+
+.marginLeft
+{
+ font: Ahem 10px;
+ height: 100px;
+ margin-left: 100px;
+ margin-right: -50px;
+ background-color: red;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+</head>
+<body>
+ <div class="hidTarget"></div>
+ <div align="center" class="sized">
+ <div class="marginLeft">xxxxx xxxxx xxxxx xxxxx xxxxx</div>
+ </div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center-expected.html (0 => 120859)
--- trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center-expected.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+/* Disable most default sizing to ensure proper position of our hidTarget. */
+body
+{
+ margin: 0px;
+}
+
+.hidTarget
+{
+ position: absolute;
+ left: 15px;
+ top: 30px;
+ height: 100px;
+ width: 100px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+<body>
+ <div class="hidTarget"></div>
+ <p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+ </p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center-expected.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center.html (0 => 120859)
--- trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center.html 2012-06-20 21:04:16 UTC (rev 120859)
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+/* Disable most default sizing to ensure proper position of our hidTarget. */
+body {
+ margin: 0px;
+}
+
+table {
+ border-spacing: 0px;
+}
+
+td {
+ padding: 0px;
+}
+
+div {
+ height: 100px;
+ width: 100px;
+}
+
+.hidTarget
+{
+ position: absolute;
+ left: 15px;
+ top: 30px;
+ background-color: green;
+}
+
+p
+{
+ position: absolute;
+ top: 300px;
+}
+</style>
+<body>
+<div class="hidTarget"></div>
+<table style="margin: 30px;">
+ <tbody>
+ <tr>
+ <td align="center">
+ <div style="margin-left: -15px; background-color: red;"></div>
+ </td>
+ </tr>
+ </tbody>
+</table>
+<p>
+ <a href="" REGRESSION(r113885): Margin not properly applied to elements with align=center<br>
+ There should be a green rectangle above with no red.
+</p>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-cell-negative-start-margin-align-center.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (120858 => 120859)
--- trunk/Source/WebCore/ChangeLog 2012-06-20 20:48:07 UTC (rev 120858)
+++ trunk/Source/WebCore/ChangeLog 2012-06-20 21:04:16 UTC (rev 120859)
@@ -1,3 +1,30 @@
+2012-06-20 Julien Chaffraix <[email protected]>
+
+ REGRESSION(r113885): Margin not properly applied to elements with align=center
+ https://bugs.webkit.org/show_bug.cgi?id=89515
+
+ Reviewed by Levi Weintraub.
+
+ Reviewed by Levi Weintraub.
+
+ Tests: fast/block/negative-margin-start-positive-margin-end.html
+ fast/block/negative-start-margin-align-center.html
+ fast/block/positive-margin-block-child-align-center-rtl.html
+ fast/block/positive-margin-block-child-align-center.html
+ fast/block/positive-margin-start-align-center.html
+ fast/block/positive-margin-start-negative-margin-end-align-center.html
+ fast/table/table-cell-negative-start-margin-align-center.html
+
+ r113885 changed the code-path for elements with auto width to call computeInlineDirectionMargins.
+ However this uncovered an existing bug in the function when dealing with align="center" (text-align: -webkit-center)
+ where we would ignore the margin. This goes against what other browsers are doing and our previous behavior.
+
+ Note that align="left" and "right" are likely impacted too and will be investigated / fixed in follow-up changes.
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::computeInlineDirectionMargins):
+ To match other browsers' behavior, changed the function to include margin in our computations.
+
2012-06-19 James Robinson <[email protected]>
[chromium] Separate LayerRenderer initialization from updateLayers
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (120858 => 120859)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-06-20 20:48:07 UTC (rev 120858)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-06-20 21:04:16 UTC (rev 120859)
@@ -1837,8 +1837,12 @@
// Case One: The object is being centered in the containing block's available logical width.
if ((marginStartLength.isAuto() && marginEndLength.isAuto() && childWidth < containerWidth)
|| (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlock->style()->textAlign() == WEBKIT_CENTER)) {
- containingBlock->setMarginStartForChild(this, max<LayoutUnit>(0, (containerWidth - childWidth) / 2));
- containingBlock->setMarginEndForChild(this, containerWidth - childWidth - containingBlock->marginStartForChild(this));
+ // Other browsers center the margin box for align=center elements so we match them here.
+ LayoutUnit marginStartWidth = marginStartLength.value();
+ LayoutUnit marginEndWidth = marginEndLength.value();
+ LayoutUnit centeredMarginBoxStart = max<LayoutUnit>(0, (containerWidth - childWidth - marginStartWidth - marginEndWidth) / 2);
+ containingBlock->setMarginStartForChild(this, centeredMarginBoxStart + marginStartWidth);
+ containingBlock->setMarginEndForChild(this, containerWidth - childWidth - containingBlock->marginStartForChild(this) + marginEndWidth);
return;
}
