Title: [120934] trunk
Revision
120934
Author
[email protected]
Date
2012-06-21 10:15:18 -0700 (Thu, 21 Jun 2012)

Log Message

Non-fixed length margins don't work with align=center
https://bugs.webkit.org/show_bug.cgi?id=89626

Reviewed by Levi Weintraub.

Source/WebCore:

Tests: fast/block/negative-start-margin-align-center-percent.html
       fast/block/positive-margin-block-child-align-center-calc.html

Calling Length::value() is a bad idea as it returns the *raw* value of
the length. For percent and calculated length this is a bad idea as they
bear not relation to the actual computed length.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computeInlineDirectionMargins):
Fixed the code to use minimumValueForLength as this nicely takes care of the 'auto' case.

LayoutTests:

* fast/block/negative-start-margin-align-center-percent-expected.html: Added.
* fast/block/negative-start-margin-align-center-percent.html: Added.
* fast/block/positive-margin-block-child-align-center-calc-expected.html: Added.
* fast/block/positive-margin-block-child-align-center-calc.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120933 => 120934)


--- trunk/LayoutTests/ChangeLog	2012-06-21 17:13:01 UTC (rev 120933)
+++ trunk/LayoutTests/ChangeLog	2012-06-21 17:15:18 UTC (rev 120934)
@@ -1,3 +1,15 @@
+2012-06-21  Julien Chaffraix  <[email protected]>
+
+        Non-fixed length margins don't work with align=center
+        https://bugs.webkit.org/show_bug.cgi?id=89626
+
+        Reviewed by Levi Weintraub.
+
+        * fast/block/negative-start-margin-align-center-percent-expected.html: Added.
+        * fast/block/negative-start-margin-align-center-percent.html: Added.
+        * fast/block/positive-margin-block-child-align-center-calc-expected.html: Added.
+        * fast/block/positive-margin-block-child-align-center-calc.html: Added.
+
 2012-06-21  Simon Pena  <[email protected]>
 
         [GTK] Update TestExpectations

Added: trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent-expected.html (0 => 120934)


--- trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent-expected.html	2012-06-21 17:15:18 UTC (rev 120934)
@@ -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="" Non-fixed length margins don't work 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-percent-expected.html
___________________________________________________________________

Added: svn:executable

Added: trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent.html (0 => 120934)


--- trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/negative-start-margin-align-center-percent.html	2012-06-21 17:15:18 UTC (rev 120934)
@@ -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: -25%;
+    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="" Non-fixed length margins don't work 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-percent.html
___________________________________________________________________

Added: svn:executable

Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc-expected.html (0 => 120934)


--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc-expected.html	2012-06-21 17:15:18 UTC (rev 120934)
@@ -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="" Non-fixed length margins don't work 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-calc-expected.html
___________________________________________________________________

Added: svn:executable

Added: trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc.html (0 => 120934)


--- trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/positive-margin-block-child-align-center-calc.html	2012-06-21 17:15:18 UTC (rev 120934)
@@ -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: -webkit-calc(20% + 60px);
+    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="" Non-fixed length margins don't work 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-calc.html
___________________________________________________________________

Added: svn:executable

Modified: trunk/Source/WebCore/ChangeLog (120933 => 120934)


--- trunk/Source/WebCore/ChangeLog	2012-06-21 17:13:01 UTC (rev 120933)
+++ trunk/Source/WebCore/ChangeLog	2012-06-21 17:15:18 UTC (rev 120934)
@@ -1,3 +1,21 @@
+2012-06-21  Julien Chaffraix  <[email protected]>
+
+        Non-fixed length margins don't work with align=center
+        https://bugs.webkit.org/show_bug.cgi?id=89626
+
+        Reviewed by Levi Weintraub.
+
+        Tests: fast/block/negative-start-margin-align-center-percent.html
+               fast/block/positive-margin-block-child-align-center-calc.html
+
+        Calling Length::value() is a bad idea as it returns the *raw* value of
+        the length. For percent and calculated length this is a bad idea as they
+        bear not relation to the actual computed length.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeInlineDirectionMargins):
+        Fixed the code to use minimumValueForLength as this nicely takes care of the 'auto' case.
+
 2012-06-21  Robert Kroeger  <[email protected]>
 
         [chromium] style improvement for setDeviceScaleFactor code

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (120933 => 120934)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2012-06-21 17:13:01 UTC (rev 120933)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2012-06-21 17:15:18 UTC (rev 120934)
@@ -1839,8 +1839,8 @@
     if ((marginStartLength.isAuto() && marginEndLength.isAuto() && childWidth < containerWidth)
         || (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlock->style()->textAlign() == WEBKIT_CENTER)) {
         // Other browsers center the margin box for align=center elements so we match them here.
-        LayoutUnit marginStartWidth = marginStartLength.value();
-        LayoutUnit marginEndWidth = marginEndLength.value();
+        LayoutUnit marginStartWidth = minimumValueForLength(marginStartLength, containerWidth, renderView);
+        LayoutUnit marginEndWidth = minimumValueForLength(marginEndLength, containerWidth, renderView);
         LayoutUnit centeredMarginBoxStart = max<LayoutUnit>(0, (containerWidth - childWidth - marginStartWidth - marginEndWidth) / 2);
         containingBlock->setMarginStartForChild(this, centeredMarginBoxStart + marginStartWidth);
         containingBlock->setMarginEndForChild(this, containerWidth - childWidth - containingBlock->marginStartForChild(this) + marginEndWidth);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to