Title: [143617] trunk
- Revision
- 143617
- Author
- abu...@adobe.com
- Date
- 2013-02-21 09:42:09 -0800 (Thu, 21 Feb 2013)
Log Message
-webkit-margin-collapse: separate doesn't work correctly for before margins
https://bugs.webkit.org/show_bug.cgi?id=109956
Reviewed by David Hyatt.
Source/WebCore:
The collapsing code for "-webkit-margin-collapse: separate" assumed the margin value inside
marginInfo always contributes to the position of the child. This is valid only if the collapse
doesn't happen at the before side of the container. In that case, the child needs to be positioned
at the margin value specified in the style sheet.
Test: fast/block/margin-collapse/webkit-margin-collapse-separate-position.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::collapseMargins):
LayoutTests:
The test verifies a child specifying "-webkit-margin-collapse: separate" is correctly positioned inside
its container, at a value equal with the top margin of the child. The container also need to be sized
accordingly.
* fast/block/margin-collapse/webkit-margin-collapse-separate-position-expected.txt: Added.
* fast/block/margin-collapse/webkit-margin-collapse-separate-position.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (143616 => 143617)
--- trunk/LayoutTests/ChangeLog 2013-02-21 17:31:34 UTC (rev 143616)
+++ trunk/LayoutTests/ChangeLog 2013-02-21 17:42:09 UTC (rev 143617)
@@ -1,3 +1,17 @@
+2013-02-21 Andrei Bucur <abu...@adobe.com>
+
+ -webkit-margin-collapse: separate doesn't work correctly for before margins
+ https://bugs.webkit.org/show_bug.cgi?id=109956
+
+ Reviewed by David Hyatt.
+
+ The test verifies a child specifying "-webkit-margin-collapse: separate" is correctly positioned inside
+ its container, at a value equal with the top margin of the child. The container also need to be sized
+ accordingly.
+
+ * fast/block/margin-collapse/webkit-margin-collapse-separate-position-expected.txt: Added.
+ * fast/block/margin-collapse/webkit-margin-collapse-separate-position.html: Added.
+
2013-02-21 Tien-Ren Chen <trc...@chromium.org>
Need to re-layout fixed position elements after scale when using settings()->fixedElementsLayoutRelativeToFrame()
Added: trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position-expected.txt (0 => 143617)
--- trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position-expected.txt 2013-02-21 17:42:09 UTC (rev 143617)
@@ -0,0 +1,11 @@
+Test for https://bugs.webkit.org/show_bug.cgi?id=109956 -webkit-margin-collapse: separate doesn't work correctly for before margins. The test basically creates an empty block inside a container with a margin that collapses with children. The empty block has the margins set to separate so its height should be 0px. The container is not selfcollapsing so it should have a height of 10px+10px+2px=22px.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.getElementById("zero_height").offsetHeight is 0
+PASS document.getElementById("22px_height").offsetHeight is 22
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position.html (0 => 143617)
--- trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position.html (rev 0)
+++ trunk/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-separate-position.html 2013-02-21 17:42:09 UTC (rev 143617)
@@ -0,0 +1,41 @@
+<html>
+ <head>
+ <script src=""
+ <style>
+ .container {
+ border: thin solid blue;
+ }
+
+ .child_with_margins {
+ margin-top: 10px;
+ margin-bottom: 10px;
+ }
+
+ .separate_top {
+ -webkit-margin-top-collapse: separate;
+ }
+
+ .separate_bottom {
+ -webkit-margin-bottom-collapse: separate;
+ }
+ </style>
+ </head>
+ <body>
+ <div class="container" id="22px_height">
+ <div class="child_with_margins" id="zero_height">
+ <div class="separate_top separate_bottom"></div>
+ </div>
+ </div>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ description("Test for https://bugs.webkit.org/show_bug.cgi?id=109956 -webkit-margin-collapse: separate doesn't work correctly for before margins. \
+ The test basically creates an empty block inside a container with a margin that collapses with children. The empty block has the margins \
+ set to separate so its height should be 0px. The container is not selfcollapsing so it should have a height of 10px+10px+2px=22px.");
+ shouldEvaluateTo('document.getElementById("zero_height").offsetHeight', '0');
+ shouldEvaluateTo('document.getElementById("22px_height").offsetHeight', '22');
+ </script>
+ <script src=""
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (143616 => 143617)
--- trunk/Source/WebCore/ChangeLog 2013-02-21 17:31:34 UTC (rev 143616)
+++ trunk/Source/WebCore/ChangeLog 2013-02-21 17:42:09 UTC (rev 143617)
@@ -1,3 +1,20 @@
+2013-02-21 Andrei Bucur <abu...@adobe.com>
+
+ -webkit-margin-collapse: separate doesn't work correctly for before margins
+ https://bugs.webkit.org/show_bug.cgi?id=109956
+
+ Reviewed by David Hyatt.
+
+ The collapsing code for "-webkit-margin-collapse: separate" assumed the margin value inside
+ marginInfo always contributes to the position of the child. This is valid only if the collapse
+ doesn't happen at the before side of the container. In that case, the child needs to be positioned
+ at the margin value specified in the style sheet.
+
+ Test: fast/block/margin-collapse/webkit-margin-collapse-separate-position.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::collapseMargins):
+
2013-02-21 Tien-Ren Chen <trc...@chromium.org>
Need to re-layout fixed position elements after scale when using settings()->fixedElementsLayoutRelativeToFrame()
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (143616 => 143617)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-02-21 17:31:34 UTC (rev 143616)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-02-21 17:42:09 UTC (rev 143617)
@@ -2078,7 +2078,11 @@
} else {
if (mustSeparateMarginBeforeForChild(child)) {
ASSERT(!marginInfo.discardMargin() || (marginInfo.discardMargin() && !marginInfo.margin()));
- setLogicalHeight(logicalHeight() + marginInfo.margin() + marginBeforeForChild(child));
+ // If we are at the before side of the block and we collapse, ignore the computed margin
+ // and just add the child margin to the container height. This will correctly position
+ // the child inside the container.
+ LayoutUnit separateMargin = !marginInfo.canCollapseWithMarginBefore() ? marginInfo.margin() : LayoutUnit(0);
+ setLogicalHeight(logicalHeight() + separateMargin + marginBeforeForChild(child));
logicalTop = logicalHeight();
} else if (!marginInfo.discardMargin() && (!marginInfo.atBeforeSideOfBlock()
|| (!marginInfo.canCollapseMarginBeforeWithChildren()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
