Implement the -webkit-margin-collapse properties correct rendering
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2013 10:22:27 +0000 (10:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2013 10:22:27 +0000 (10:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108168

Patch by Andrei Bucur <abucur@adobe.com> on 2013-02-15
Reviewed by David Hyatt.

Source/WebCore:

The patch implements the correct behavior for the -webkit-margin-collapse properties:
- a value of "discard" on a margin will truncate all the margins collapsing with it;
- a value of "separate" will prevent the margin to collapse;
- a value of "collapse" is the default collapse behavior.

The implementation is aware of multiple writing-modes:
- if the writing mode of a child is parallel with the writing mode of the container and has the same direction,
the -webkit-margin-collapse properties on the child are left as is;
- if the writing mode of a child is parallel with the writing mode of the container but has a different direction,
the -webkit-margin-collapse properties on the child are reversed;
- if the writing mode of a child is perpendicular on the writing mode of the container,
the -webkit-margin-collapse properties on the child are ignored;

I. The "discard" value implementation
There are two new bits (before and after) added on the RenderBlockRareData structure specifying if the margins
of the block will be discarded or not. We can't rely only on the value from style() because
it's possible a block to discard it's margins because it has collapsed with a children that
specified "discard" for -webkit-margin-collapse. However, the bits are set only if it is
required.
Another bit is added on the MarginInfo structure specifying if the margin has to be discarded or not. When
collapsing at the before side of a block it will hold information if the container block needs to discard
or not. If the collapsing happens between siblings/with after side of the container it will tell if the previous
child discards the margin or not. The self collapsing blocks are a special case. If any of its margins
discards then both its margins discard and all the other margins collapsing with it.
To ensure an optimal behavior it is asserted margin values can't be set on the MarginInfo object if the
discard flag is active. If this happens it may indicate someone ignored the possibility of the margin being
discarded altogether and incorrectly updated the margin values.
Float clearing also needs to change because it may force margins to stop collapsing. If this happens the discard
flags and margins needs to be restored to their values before the collapse.

II. The "separate" value implementation
The implementation for separate was not changed too much. I've added new accessor methods for the property
that take writing mode into consideration and I've removed some code that didn't work correctly in layoutBlockChild.
The problem was the marginInfo structure was cleared if the child was specifying the "separate" value for before.
This is wrong because you lose the margin information of the previous child/before side.

Tests: fast/block/margin-collapse/webkit-margin-collapse-container.html
       fast/block/margin-collapse/webkit-margin-collapse-floats.html
       fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html
       fast/block/margin-collapse/webkit-margin-collapse-siblings.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::MarginInfo::MarginInfo):
(WebCore::RenderBlock::layoutBlock):
(WebCore::RenderBlock::collapseMargins):
(WebCore::RenderBlock::clearFloatsIfNeeded):
(WebCore::RenderBlock::marginBeforeEstimateForChild):
(WebCore::RenderBlock::estimateLogicalTopPosition):
(WebCore::RenderBlock::setCollapsedBottomMargin):
(WebCore::RenderBlock::handleAfterSideOfBlock):
(WebCore::RenderBlock::layoutBlockChild):
(WebCore::RenderBlock::setMustDiscardMarginBefore):
(WebCore):
(WebCore::RenderBlock::setMustDiscardMarginAfter):
(WebCore::RenderBlock::mustDiscardMarginBefore):
(WebCore::RenderBlock::mustDiscardMarginAfter):
(WebCore::RenderBlock::mustDiscardMarginBeforeForChild):
(WebCore::RenderBlock::mustDiscardMarginAfterForChild):
(WebCore::RenderBlock::mustSeparateMarginBeforeForChild):
(WebCore::RenderBlock::mustSeparateMarginAfterForChild):
* rendering/RenderBlock.h:
(RenderBlock):
(WebCore::RenderBlock::initMaxMarginValues):
(MarginInfo):
(WebCore::RenderBlock::MarginInfo::setPositiveMargin):
(WebCore::RenderBlock::MarginInfo::setNegativeMargin):
(WebCore::RenderBlock::MarginInfo::setPositiveMarginIfLarger):
(WebCore::RenderBlock::MarginInfo::setNegativeMarginIfLarger):
(WebCore::RenderBlock::MarginInfo::setMargin):
(WebCore::RenderBlock::MarginInfo::setCanCollapseMarginAfterWithChildren):
(WebCore::RenderBlock::MarginInfo::setDiscardMargin):
(WebCore::RenderBlock::MarginInfo::discardMargin):
(WebCore::RenderBlock::RenderBlockRareData::RenderBlockRareData):
(WebCore::RenderBlock::RenderBlockRareData::positiveMarginBeforeDefault):
(RenderBlockRareData):
* rendering/style/RenderStyle.h:

LayoutTests:

Four new tests covering the -webkit-margin-collapse property basic behavior: collapsing
between a block container and its children, collapsing between sibling boxes in both TTB
and BTT direction. The last test verifies if a container's before margin correctly resets
the discard value after a clear of the child that initally caused it.

* fast/block/margin-collapse/webkit-margin-collapse-container-expected.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-container.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-floats-expected.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-floats.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-siblings-bt-expected.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-siblings-expected.html: Added.
* fast/block/margin-collapse/webkit-margin-collapse-siblings.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142974 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container-expected.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats-expected.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt-expected.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-expected.html [new file with mode: 0644]
LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/style/RenderStyle.h

index 35cf5316d2b8fc09e7e4ccb932167fdfd2f2ddfe..090725beba79231f232a7b0c3238c8ca31d82b44 100644 (file)
@@ -1,3 +1,24 @@
+2013-02-15  Andrei Bucur  <abucur@adobe.com>
+
+        Implement the -webkit-margin-collapse properties correct rendering
+        https://bugs.webkit.org/show_bug.cgi?id=108168
+
+        Reviewed by David Hyatt.
+
+        Four new tests covering the -webkit-margin-collapse property basic behavior: collapsing
+        between a block container and its children, collapsing between sibling boxes in both TTB
+        and BTT direction. The last test verifies if a container's before margin correctly resets
+        the discard value after a clear of the child that initally caused it.
+
+        * fast/block/margin-collapse/webkit-margin-collapse-container-expected.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-container.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-floats-expected.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-floats.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-siblings-bt-expected.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-siblings-expected.html: Added.
+        * fast/block/margin-collapse/webkit-margin-collapse-siblings.html: Added.
+
 2013-02-15  KwangYong Choi  <ky0.choi@samsung.com>
 
         [EFL] fast/forms/input-text-scroll-left-on-blur.html is passing now
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container-expected.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container-expected.html
new file mode 100644 (file)
index 0000000..02e3285
--- /dev/null
@@ -0,0 +1,159 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 350px;
+                       float: left;
+               }
+
+               .level1 {
+                       margin-top: 10px;
+                       margin-bottom: 10px;
+                       border: 1px dashed black;
+                       background-color: green;
+               }
+
+               .level2 {
+                       margin-top: 25px;
+                       margin-bottom: 20px;
+                       background-color: orange;
+               }
+
+               .level3 {
+                       margin-top: 30px;
+                       margin-bottom: 30px;
+                       background-color: pink;
+               }
+
+               #t1 .level2 {
+                       margin-top: 0px;
+               }
+
+               #t1 .level3 {
+                       margin-top: 0px;
+               }
+
+               #t2 .level2 {
+                       margin-bottom: 0px;
+               }
+
+               #t2 .level3 {
+                       margin-bottom: 0px;
+               }
+
+               #t3 .level2 {
+                       margin-top: 0px;
+               }
+
+               #t3 .level3 {
+                       margin-top: 0px;
+               }
+
+               #t4 .level2 {
+                       margin-bottom: 0px;
+               }
+
+               #t4 .level3 {
+                       margin-bottom: 0px;
+               }
+
+               #t5 .level2 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #t5 .level3 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #t6 .level2 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #t6 .level3 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #t7 .level3 {
+                       margin-top: 0px;
+               }
+
+               #t8 .level3 {
+                       margin-bottom: 0px;
+               }
+
+               #t9 .level2 {
+                       margin-top: 0px;
+                       padding-top: 30px;
+               }
+
+               #t9 .level3 {
+                       margin-top: 0px;
+               }
+
+               #t10 .level2 {
+                       margin-bottom: 0px;
+                       padding-bottom: 30px;
+               }
+
+               #t10 .level3 {
+                       margin-bottom: 0px;
+               }
+
+               #t11 .level2 {
+                       margin-top: 0px;
+                       padding-top: 30px;
+               }
+
+               #t11 .level3 {
+                       margin-top: 0px;
+               }
+
+               #t11 {
+                       padding-top: 25px;
+               }
+
+               #t12 .level2 {
+                       margin-bottom: 0px;
+                       padding-bottom: 30px;
+               }
+
+               #t12 .level3 {
+                       margin-bottom: 0px;
+               }
+
+               #t12 {
+                       padding-bottom: 20px;
+               }
+
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between a box and its container when using -webkit-margin-collapse.</p>
+               <div class="container">
+                       <div id="t1" class="level1"><div class="level2"><div class="level3">Pink: discard top</div></div></div>
+                       <div id="t2" class="level1"><div class="level2"><div class="level3">Pink: discard bottom</div></div></div>
+                       <div id="t3" class="level1"><div class="level2"><div class="level3">Orange: discard top</div></div></div>
+                       <div id="t4" class="level1"><div class="level2"><div class="level3">Orange: discard bottom</div></div></div>
+                       <div id="t5" class="level1"><div class="level2"><div class="level3">Orange: discard top, discard bottom</div></div></div>
+                       <div id="t6" class="level1"><div class="level2"><div class="level3">Pink: discard top, discard bottom</div></div></div>
+                       <div id="t7" class="level1"><div class="level2"><div class="level3">Orange: separate top, Pink: discard top</div></div></div>
+                       <div id="t8" class="level1"><div class="level2"><div class="level3">Orange: separate bottom, Pink: discard bottom</div></div></div>
+               </div>
+               <div class="container">
+                       <div id="t9" class="level1"><div class="level2"><div class="level3">Orange: discard top, Pink: separate top</div></div></div>
+                       <div id="t10" class="level1"><div class="level2"><div class="level3">Orange: discard bottom, Pink: separate bottom</div></div></div>
+                       <div id="t11" class="level1"><div class="level2"><div class="level3">Orange: separate top, Pink: separate top</div></div></div>
+                       <div id="t12" class="level1"><div class="level2"><div class="level3">Orange: separate bottom, Pink: separate bottom</div></div></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-container.html
new file mode 100644 (file)
index 0000000..63d86ca
--- /dev/null
@@ -0,0 +1,78 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 350px;
+                       float: left;
+               }
+
+               .level1 {
+                       margin-top: 10px;
+                       margin-bottom: 10px;
+                       border: 1px dashed black;
+                       background-color: green;
+               }
+
+               .level2 {
+                       margin-top: 25px;
+                       margin-bottom: 20px;
+                       background-color: orange;
+               }
+
+               .level3 {
+                       margin-top: 30px;
+                       margin-bottom: 30px;
+                       background-color: pink;
+               }
+
+               .collapse_top {
+                       -webkit-margin-top-collapse: collapse;
+               }
+
+               .collapse_bottom {
+                       -webkit-margin-bottom-collapse: collapse;
+               }
+
+               .discard_top {
+                       -webkit-margin-top-collapse: discard;
+               }
+
+               .discard_bottom {
+                       -webkit-margin-bottom-collapse: discard;
+               }
+
+               .separate_top {
+                       -webkit-margin-top-collapse: separate;
+               }
+
+               .separate_bottom {
+                       -webkit-margin-bottom-collapse: separate;
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between a box and its container when using -webkit-margin-collapse.</p>
+               <div class="container">
+                       <div class="level1"><div class="level2"><div class="level3 discard_top">Pink: discard top</div></div></div>
+                       <div class="level1"><div class="level2"><div class="level3 discard_bottom">Pink: discard bottom</div></div></div>
+                       <div class="level1"><div class="level2 discard_top"><div class="level3">Orange: discard top</div></div></div>
+                       <div class="level1"><div class="level2 discard_bottom"><div class="level3">Orange: discard bottom</div></div></div>
+                       <div class="level1"><div class="level2 discard_top discard_bottom"><div class="level3">Orange: discard top, discard bottom</div></div></div>
+                       <div class="level1"><div class="level2"><div class="level3 discard_top discard_bottom">Pink: discard top, discard bottom</div></div></div>
+                       <div class="level1"><div class="level2 separate_top"><div class="level3 discard_top">Orange: separate top, Pink: discard top</div></div></div>
+                       <div class="level1"><div class="level2 separate_bottom"><div class="level3 discard_bottom">Orange: separate bottom, Pink: discard bottom</div></div></div>
+               </div>
+               <div class="container">
+                       <div class="level1"><div class="level2 discard_top"><div class="level3 separate_top">Orange: discard top, Pink: separate top</div></div></div>
+                       <div class="level1"><div class="level2 discard_bottom"><div class="level3 separate_bottom">Orange: discard bottom, Pink: separate bottom</div></div></div>
+                       <div class="level1"><div class="level2 separate_top"><div class="level3 separate_top">Orange: separate top, Pink: separate top</div></div></div>
+                       <div class="level1"><div class="level2 separate_bottom"><div class="level3 separate_bottom">Orange: separate bottom, Pink: separate bottom</div></div></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats-expected.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats-expected.html
new file mode 100644 (file)
index 0000000..6ca1d39
--- /dev/null
@@ -0,0 +1,41 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 600px;
+               }
+
+               #float {
+                       float: left;
+                       width: 200px;
+                       height: 200px;
+                       background-color: green;
+               }
+
+               #parent {
+                       margin-top: 50px;
+               }
+
+               #content {
+                       margin-top: 50px;
+                       clear: left;
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior when a child with a discard top margin is cleared and its top margin doesn't collapse with the top margin of the containing block any more.
+                       The top margin of the containing block needs to be restored.</p>
+               <div class="container">
+                       <div id="parent">
+                               <div id="float"></div>
+                               <div id="content">Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nunc orci mauris, ultrices non blandit non, fermentum a libero. Morbi ante elit, faucibus vel dignissim quis, euismod a arcu. Sed tempus suscipit nulla, vitae varius elit suscipit et. Pellentesque consequat lobortis blandit. Vivamus a tempus diam. Suspendisse nulla lectus, consequat quis tempor vitae, placerat sed lorem. Phasellus at dolor felis, ut ultricies lacus. Praesent vel turpis lacus. Nam ultricies augue in erat congue ac tempor neque cursus.</div>
+                       </div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-floats.html
new file mode 100644 (file)
index 0000000..af9a49a
--- /dev/null
@@ -0,0 +1,45 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 600px;
+               }
+
+               .discard_top {
+                       -webkit-margin-top-collapse: discard;
+               }
+
+               #float {
+                       float: left;
+                       width: 200px;
+                       height: 200px;
+                       background-color: green;
+               }
+
+               #parent {
+                       margin-top: 50px;
+               }
+
+               #content {
+                       margin-top: 50px;
+                       clear: left;
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior when a child with a discard top margin is cleared and its top margin doesn't collapse with the top margin of the containing block any more.
+                       The top margin of the containing block needs to be restored.</p>
+               <div class="container">
+                       <div id="parent">
+                               <div id="float"></div>
+                               <div id="content" class="discard_top">Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nunc orci mauris, ultrices non blandit non, fermentum a libero. Morbi ante elit, faucibus vel dignissim quis, euismod a arcu. Sed tempus suscipit nulla, vitae varius elit suscipit et. Pellentesque consequat lobortis blandit. Vivamus a tempus diam. Suspendisse nulla lectus, consequat quis tempor vitae, placerat sed lorem. Phasellus at dolor felis, ut ultricies lacus. Praesent vel turpis lacus. Nam ultricies augue in erat congue ac tempor neque cursus.</div>
+                       </div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt-expected.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt-expected.html
new file mode 100644 (file)
index 0000000..1b53d25
--- /dev/null
@@ -0,0 +1,95 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 300px;
+               }
+
+               .block {
+                       background-color: green;
+                       margin-top: 4px;
+                       margin-bottom: 6px;
+                       border: 1px dashed black;
+                       -webkit-writing-mode: horizontal-bt;
+               }
+
+               .collapse_top:before, .collapse_bottom:after {
+                       content: "Collapse";
+               }
+
+               .discard_top:before, .discard_bottom:after {
+                       content: "Discard";
+               }
+
+               .separate_top:before, .separate_bottom:after {
+                       content: "Separate";
+               }
+
+               #b2 {
+                       margin-top: 6px;
+                       margin-bottom: 0px;
+               }
+
+               #b3 {
+                       margin-top: 6px;
+                       margin-bottom: 0px;
+               }
+
+               #b4 {
+                       margin-top: 6px;
+                       margin-bottom: 0px;
+               }
+
+               #b5 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #b6 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #b7 {
+                       margin-top: 14px;
+                       margin-bottom: 0px;
+               }
+
+               #b8 {
+                       margin-top: 4px;
+                       margin-bottom: 0px;
+               }
+
+               #b9 {
+                       margin-top: 10px;
+                       margin-bottom: 10px;
+               }
+
+               #b10 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between sibling boxes when using -webkit-margin-collapse. The blocks have lr-bt writing mode.</p>
+               <div class="container">
+                       <div id="b1" class="block collapse_top collapse_bottom"><br/></div>
+                       <div id="b2" class="block collapse_top collapse_bottom"><br/></div>
+                       <div id="b3" class="block separate_top collapse_bottom"><br/></div>
+                       <div id="b4" class="block discard_top discard_bottom"><br/></div>
+                       <div id="b5" class="block collapse_top discard_bottom"><br/></div>
+                       <div id="b6" class="block separate_top discard_bottom"><br/></div>
+                       <div id="b7" class="block discard_top separate_bottom"><br/></div>
+                       <div id="b8" class="block collapse_top separate_bottom"><br/></div>
+                       <div id="b9" class="block separate_top separate_bottom"><br/></div>
+                       <div id="b10" class="block discard_top collapse_bottom"><br/></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html
new file mode 100644 (file)
index 0000000..f8a30e1
--- /dev/null
@@ -0,0 +1,89 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 300px;
+               }
+
+               .block {
+                       background-color: green;
+                       margin-top: 4px;
+                       margin-bottom: 6px;
+                       border: 1px dashed black;
+                       -webkit-writing-mode: horizontal-bt;
+               }
+
+               .self_collapse {
+                       margin-top: 2px;
+                       margin-bottom: 4px;
+                       height: 0px;
+               }
+
+               .collapse_top {
+                       -webkit-margin-top-collapse: collapse;
+               }
+
+               .collapse_bottom {
+                       -webkit-margin-bottom-collapse: collapse;
+               }
+
+               .collapse_top:before, .collapse_bottom:after {
+                       content: "Collapse";
+               }
+
+               .discard_top {
+                       -webkit-margin-top-collapse: discard;
+               }
+
+               .discard_bottom {
+                       -webkit-margin-bottom-collapse: discard;
+               }
+
+               .discard_top:before, .discard_bottom:after {
+                       content: "Discard";
+               }
+
+               .separate_top {
+                       -webkit-margin-top-collapse: separate;
+               }
+
+               .separate_bottom {
+                       -webkit-margin-bottom-collapse: separate;
+               }
+
+               .separate_top:before, .separate_bottom:after {
+                       content: "Separate";
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between sibling boxes when using -webkit-margin-collapse. The blocks have lr-bt writing mode.</p>
+               <div class="container">
+                       <div class="block collapse_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top collapse_bottom"><br/></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-expected.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings-expected.html
new file mode 100644 (file)
index 0000000..72fd293
--- /dev/null
@@ -0,0 +1,93 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 300px;
+               }
+
+               .block {
+                       background-color: green;
+                       margin-top: 4px;
+                       margin-bottom: 6px;
+                       border: 1px dashed black;
+               }
+
+               .collapse_top:before, .collapse_bottom:after {
+                       content: "Collapse";
+               }
+
+               .discard_top:before, .discard_bottom:after {
+                       content: "Discard";
+               }
+
+               .separate_top:before, .separate_bottom:after {
+                       content: "Separate";
+               }
+
+               #b2 {
+                       margin-top: 6px;
+                       margin-bottom: 0px;
+               }
+
+               #b3 {
+                       margin-top: 10px;
+                       margin-bottom: 0px;
+               }
+
+               #b4 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #b5 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #b6 {
+                       margin-top: 4px;
+                       margin-bottom: 0px;
+               }
+
+               #b7 {
+                       margin-top: 0px;
+                       margin-bottom: 0px;
+               }
+
+               #b8 {
+                       margin-top: 10px;
+                       margin-bottom: 0px;
+               }
+
+               #b9 {
+                       margin-top: 14px;
+                       margin-bottom: 6px;
+               }
+
+               #b10 {
+                       margin-top: 0px;
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between sibling boxes when using -webkit-margin-collapse. The blocks have lr-tb writing mode.</p>
+               <div class="container">
+                       <div id="b1" class="block collapse_top collapse_bottom"><br/></div>
+                       <div id="b2" class="block collapse_top collapse_bottom"><br/></div>
+                       <div id="b3" class="block separate_top collapse_bottom"><br/></div>
+                       <div id="b4" class="block discard_top discard_bottom"><br/></div>
+                       <div id="b5" class="block collapse_top discard_bottom"><br/></div>
+                       <div id="b6" class="block separate_top discard_bottom"><br/></div>
+                       <div id="b7" class="block discard_top separate_bottom"><br/></div>
+                       <div id="b8" class="block collapse_top separate_bottom"><br/></div>
+                       <div id="b9" class="block separate_top separate_bottom"><br/></div>
+                       <div id="b10" class="block discard_top collapse_bottom"><br/></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings.html b/LayoutTests/fast/block/margin-collapse/webkit-margin-collapse-siblings.html
new file mode 100644 (file)
index 0000000..ab6cd88
--- /dev/null
@@ -0,0 +1,88 @@
+<html>
+       <head>
+               <style>
+               div {
+                       margin: 0px;
+               }
+
+               .container {
+                       border: thin solid blue;
+                       width: 300px;
+               }
+
+               .block {
+                       background-color: green;
+                       margin-top: 4px;
+                       margin-bottom: 6px;
+                       border: 1px dashed black;
+               }
+
+               .self_collapse {
+                       margin-top: 2px;
+                       margin-bottom: 4px;
+                       height: 0px;
+               }
+
+               .collapse_top {
+                       -webkit-margin-top-collapse: collapse;
+               }
+
+               .collapse_bottom {
+                       -webkit-margin-bottom-collapse: collapse;
+               }
+
+               .collapse_top:before, .collapse_bottom:after {
+                       content: "Collapse";
+               }
+
+               .discard_top {
+                       -webkit-margin-top-collapse: discard;
+               }
+
+               .discard_bottom {
+                       -webkit-margin-bottom-collapse: discard;
+               }
+
+               .discard_top:before, .discard_bottom:after {
+                       content: "Discard";
+               }
+
+               .separate_top {
+                       -webkit-margin-top-collapse: separate;
+               }
+
+               .separate_bottom {
+                       -webkit-margin-bottom-collapse: separate;
+               }
+
+               .separate_top:before, .separate_bottom:after {
+                       content: "Separate";
+               }
+               </style>
+       </head>
+       <body>
+               <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108168">Implement the -webkit-margin-collapse properties correct rendering</a>.</p>
+               <p>This ref test covers basic collapsing behavior between sibling boxes when using -webkit-margin-collapse. The blocks have lr-tb writing mode.</p>
+               <div class="container">
+                       <div class="block collapse_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top collapse_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top discard_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block collapse_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block separate_top separate_bottom"><br/></div>
+                       <div class="self_collapse"></div>
+                       <div class="block discard_top collapse_bottom"><br/></div>
+               </div>
+       </body>
+</html>
\ No newline at end of file
index 493e5e5a617a7e43c2dc6a5b40b0a7eff4690b27..0f0134f193b185bd6db075067c4ac54b574549cb 100644 (file)
@@ -1,3 +1,87 @@
+2013-02-15  Andrei Bucur  <abucur@adobe.com>
+
+        Implement the -webkit-margin-collapse properties correct rendering
+        https://bugs.webkit.org/show_bug.cgi?id=108168
+
+        Reviewed by David Hyatt.
+
+        The patch implements the correct behavior for the -webkit-margin-collapse properties:
+        - a value of "discard" on a margin will truncate all the margins collapsing with it;
+        - a value of "separate" will prevent the margin to collapse;
+        - a value of "collapse" is the default collapse behavior.
+
+        The implementation is aware of multiple writing-modes:
+        - if the writing mode of a child is parallel with the writing mode of the container and has the same direction,
+        the -webkit-margin-collapse properties on the child are left as is;
+        - if the writing mode of a child is parallel with the writing mode of the container but has a different direction,
+        the -webkit-margin-collapse properties on the child are reversed;
+        - if the writing mode of a child is perpendicular on the writing mode of the container,
+        the -webkit-margin-collapse properties on the child are ignored;
+
+        I. The "discard" value implementation
+        There are two new bits (before and after) added on the RenderBlockRareData structure specifying if the margins
+        of the block will be discarded or not. We can't rely only on the value from style() because
+        it's possible a block to discard it's margins because it has collapsed with a children that
+        specified "discard" for -webkit-margin-collapse. However, the bits are set only if it is
+        required.
+        Another bit is added on the MarginInfo structure specifying if the margin has to be discarded or not. When
+        collapsing at the before side of a block it will hold information if the container block needs to discard
+        or not. If the collapsing happens between siblings/with after side of the container it will tell if the previous
+        child discards the margin or not. The self collapsing blocks are a special case. If any of its margins
+        discards then both its margins discard and all the other margins collapsing with it.
+        To ensure an optimal behavior it is asserted margin values can't be set on the MarginInfo object if the
+        discard flag is active. If this happens it may indicate someone ignored the possibility of the margin being
+        discarded altogether and incorrectly updated the margin values.
+        Float clearing also needs to change because it may force margins to stop collapsing. If this happens the discard
+        flags and margins needs to be restored to their values before the collapse.
+
+        II. The "separate" value implementation
+        The implementation for separate was not changed too much. I've added new accessor methods for the property
+        that take writing mode into consideration and I've removed some code that didn't work correctly in layoutBlockChild.
+        The problem was the marginInfo structure was cleared if the child was specifying the "separate" value for before.
+        This is wrong because you lose the margin information of the previous child/before side.
+
+        Tests: fast/block/margin-collapse/webkit-margin-collapse-container.html
+               fast/block/margin-collapse/webkit-margin-collapse-floats.html
+               fast/block/margin-collapse/webkit-margin-collapse-siblings-bt.html
+               fast/block/margin-collapse/webkit-margin-collapse-siblings.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::MarginInfo::MarginInfo):
+        (WebCore::RenderBlock::layoutBlock):
+        (WebCore::RenderBlock::collapseMargins):
+        (WebCore::RenderBlock::clearFloatsIfNeeded):
+        (WebCore::RenderBlock::marginBeforeEstimateForChild):
+        (WebCore::RenderBlock::estimateLogicalTopPosition):
+        (WebCore::RenderBlock::setCollapsedBottomMargin):
+        (WebCore::RenderBlock::handleAfterSideOfBlock):
+        (WebCore::RenderBlock::layoutBlockChild):
+        (WebCore::RenderBlock::setMustDiscardMarginBefore):
+        (WebCore):
+        (WebCore::RenderBlock::setMustDiscardMarginAfter):
+        (WebCore::RenderBlock::mustDiscardMarginBefore):
+        (WebCore::RenderBlock::mustDiscardMarginAfter):
+        (WebCore::RenderBlock::mustDiscardMarginBeforeForChild):
+        (WebCore::RenderBlock::mustDiscardMarginAfterForChild):
+        (WebCore::RenderBlock::mustSeparateMarginBeforeForChild):
+        (WebCore::RenderBlock::mustSeparateMarginAfterForChild):
+        * rendering/RenderBlock.h:
+        (RenderBlock):
+        (WebCore::RenderBlock::initMaxMarginValues):
+        (MarginInfo):
+        (WebCore::RenderBlock::MarginInfo::setPositiveMargin):
+        (WebCore::RenderBlock::MarginInfo::setNegativeMargin):
+        (WebCore::RenderBlock::MarginInfo::setPositiveMarginIfLarger):
+        (WebCore::RenderBlock::MarginInfo::setNegativeMarginIfLarger):
+        (WebCore::RenderBlock::MarginInfo::setMargin):
+        (WebCore::RenderBlock::MarginInfo::setCanCollapseMarginAfterWithChildren):
+        (WebCore::RenderBlock::MarginInfo::setDiscardMargin):
+        (WebCore::RenderBlock::MarginInfo::discardMargin):
+        (WebCore::RenderBlock::RenderBlockRareData::RenderBlockRareData):
+        (WebCore::RenderBlock::RenderBlockRareData::positiveMarginBeforeDefault):
+        (RenderBlockRareData):
+        * rendering/style/RenderStyle.h:
+
 2013-02-14  Yury Semikhatsky  <yurys@chromium.org>
 
         Web Inspector: always show memory size in Mb on the native memory graph
index b105d60ca99906aeb5c37916dce8741c0c1a3394..02926738beb56f6807d200ad54061062902ab9ab 100644 (file)
@@ -169,6 +169,7 @@ RenderBlock::MarginInfo::MarginInfo(RenderBlock* block, LayoutUnit beforeBorderP
     , m_marginBeforeQuirk(false)
     , m_marginAfterQuirk(false)
     , m_determinedMarginBeforeQuirk(false)
+    , m_discardMargin(false)
 {
     RenderStyle* blockStyle = block->style();
     ASSERT(block->isRenderView() || block->parent());
@@ -186,11 +187,12 @@ RenderBlock::MarginInfo::MarginInfo(RenderBlock* block, LayoutUnit beforeBorderP
     m_canCollapseMarginAfterWithChildren = m_canCollapseWithChildren && (afterBorderPadding == 0) &&
         (blockStyle->logicalHeight().isAuto() && !blockStyle->logicalHeight().value()) && blockStyle->marginAfterCollapse() != MSEPARATE;
     
-    m_quirkContainer = block->isTableCell() || block->isBody() || blockStyle->marginBeforeCollapse() == MDISCARD
-        || blockStyle->marginAfterCollapse() == MDISCARD;
+    m_quirkContainer = block->isTableCell() || block->isBody();
 
-    m_positiveMargin = m_canCollapseMarginBeforeWithChildren ? block->maxPositiveMarginBefore() : LayoutUnit();
-    m_negativeMargin = m_canCollapseMarginBeforeWithChildren ? block->maxNegativeMarginBefore() : LayoutUnit();
+    m_discardMargin = m_canCollapseMarginBeforeWithChildren && block->mustDiscardMarginBefore();
+
+    m_positiveMargin = (m_canCollapseMarginBeforeWithChildren && !block->mustDiscardMarginBefore()) ? block->maxPositiveMarginBefore() : LayoutUnit();
+    m_negativeMargin = (m_canCollapseMarginBeforeWithChildren && !block->mustDiscardMarginBefore()) ? block->maxNegativeMarginBefore() : LayoutUnit();
 }
 
 // -------------------------------------------------------------------------------------------------------
@@ -1526,8 +1528,8 @@ void RenderBlock::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeigh
     if (!isCell) {
         initMaxMarginValues();
         
-        setMarginBeforeQuirk(styleToUse->marginBefore().quirk());
-        setMarginAfterQuirk(styleToUse->marginAfter().quirk());
+        setMarginBeforeQuirk(styleToUse->isMarginBeforeQuirk());
+        setMarginAfterQuirk(styleToUse->isMarginAfterQuirk());
         setPaginationStrut(0);
     }
 
@@ -1957,6 +1959,13 @@ void RenderBlock::moveRunInToOriginalPosition(RenderObject* runIn)
 
 LayoutUnit RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo)
 {
+    bool childDiscardMarginBefore = mustDiscardMarginBeforeForChild(child);
+    bool childDiscardMarginAfter = mustDiscardMarginAfterForChild(child);
+    bool childIsSelfCollapsing = child->isSelfCollapsingBlock();
+
+    // The child discards the before margin when the the after margin has discard in the case of a self collapsing block.
+    childDiscardMarginBefore = childDiscardMarginBefore || (childDiscardMarginAfter && childIsSelfCollapsing);
+
     // Get the four margin values for the child and cache them.
     const MarginValues childMargins = marginValuesForChild(child);
 
@@ -1966,38 +1975,48 @@ LayoutUnit RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo
 
     // For self-collapsing blocks, collapse our bottom margins into our
     // top to get new posTop and negTop values.
-    if (child->isSelfCollapsingBlock()) {
+    if (childIsSelfCollapsing) {
         posTop = max(posTop, childMargins.positiveMarginAfter());
         negTop = max(negTop, childMargins.negativeMarginAfter());
     }
     
     // See if the top margin is quirky. We only care if this child has
     // margins that will collapse with us.
-    bool topQuirk = child->isMarginBeforeQuirk() || style()->marginBeforeCollapse() == MDISCARD;
+    bool topQuirk = child->isMarginBeforeQuirk();
 
     if (marginInfo.canCollapseWithMarginBefore()) {
-        // This child is collapsing with the top of the
-        // block.  If it has larger margin values, then we need to update
-        // our own maximal values.
-        if (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !topQuirk)
-            setMaxMarginBeforeValues(max(posTop, maxPositiveMarginBefore()), max(negTop, maxNegativeMarginBefore()));
-
-        // The minute any of the margins involved isn't a quirk, don't
-        // collapse it away, even if the margin is smaller (www.webreference.com
-        // has an example of this, a <dt> with 0.8em author-specified inside
-        // a <dl> inside a <td>.
-        if (!marginInfo.determinedMarginBeforeQuirk() && !topQuirk && (posTop - negTop)) {
-            setMarginBeforeQuirk(false);
-            marginInfo.setDeterminedMarginBeforeQuirk(true);
-        }
-
-        if (!marginInfo.determinedMarginBeforeQuirk() && topQuirk && !marginBefore())
-            // We have no top margin and our top child has a quirky margin.
-            // We will pick up this quirky margin and pass it through.
-            // This deals with the <td><div><p> case.
-            // Don't do this for a block that split two inlines though.  You do
-            // still apply margins in this case.
-            setMarginBeforeQuirk(true);
+        if (!childDiscardMarginBefore && !marginInfo.discardMargin()) {
+            // This child is collapsing with the top of the
+            // block. If it has larger margin values, then we need to update
+            // our own maximal values.
+            if (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !topQuirk)
+                setMaxMarginBeforeValues(max(posTop, maxPositiveMarginBefore()), max(negTop, maxNegativeMarginBefore()));
+
+            // The minute any of the margins involved isn't a quirk, don't
+            // collapse it away, even if the margin is smaller (www.webreference.com
+            // has an example of this, a <dt> with 0.8em author-specified inside
+            // a <dl> inside a <td>.
+            if (!marginInfo.determinedMarginBeforeQuirk() && !topQuirk && (posTop - negTop)) {
+                setMarginBeforeQuirk(false);
+                marginInfo.setDeterminedMarginBeforeQuirk(true);
+            }
+
+            if (!marginInfo.determinedMarginBeforeQuirk() && topQuirk && !marginBefore())
+                // We have no top margin and our top child has a quirky margin.
+                // We will pick up this quirky margin and pass it through.
+                // This deals with the <td><div><p> case.
+                // Don't do this for a block that split two inlines though. You do
+                // still apply margins in this case.
+                setMarginBeforeQuirk(true);
+        } else
+            // The before margin of the container will also discard all the margins it is collapsing with.
+            setMustDiscardMarginBefore();
+    }
+
+    // Once we find a child with discardMarginBefore all the margins collapsing with us must also discard. 
+    if (childDiscardMarginBefore) {
+        marginInfo.setDiscardMargin(true);
+        marginInfo.clearMargin();
     }
 
     if (marginInfo.quirkContainer() && marginInfo.atBeforeSideOfBlock() && (posTop - negTop))
@@ -2005,45 +2024,53 @@ LayoutUnit RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo
 
     LayoutUnit beforeCollapseLogicalTop = logicalHeight();
     LayoutUnit logicalTop = beforeCollapseLogicalTop;
-    if (child->isSelfCollapsingBlock()) {
-        // This child has no height.  We need to compute our
-        // position before we collapse the child's margins together,
-        // so that we can get an accurate position for the zero-height block.
-        LayoutUnit collapsedBeforePos = max(marginInfo.positiveMargin(), childMargins.positiveMarginBefore());
-        LayoutUnit collapsedBeforeNeg = max(marginInfo.negativeMargin(), childMargins.negativeMarginBefore());
-        marginInfo.setMargin(collapsedBeforePos, collapsedBeforeNeg);
-        
-        // Now collapse the child's margins together, which means examining our
-        // bottom margin values as well. 
-        marginInfo.setPositiveMarginIfLarger(childMargins.positiveMarginAfter());
-        marginInfo.setNegativeMarginIfLarger(childMargins.negativeMarginAfter());
-
-        if (!marginInfo.canCollapseWithMarginBefore())
-            // We need to make sure that the position of the self-collapsing block
-            // is correct, since it could have overflowing content
-            // that needs to be positioned correctly (e.g., a block that
-            // had a specified height of 0 but that actually had subcontent).
-            logicalTop = logicalHeight() + collapsedBeforePos - collapsedBeforeNeg;
-    }
-    else {
-        if (child->style()->marginBeforeCollapse() == MSEPARATE) {
+    if (childIsSelfCollapsing) {
+        // For a self collapsing block both the before and after margins get discarded. The block doesn't contribute anything to the height of the block.
+        // Also, the child's top position equals the logical height of the container.
+        if (!childDiscardMarginBefore && !marginInfo.discardMargin()) {
+            // This child has no height. We need to compute our
+            // position before we collapse the child's margins together,
+            // so that we can get an accurate position for the zero-height block.
+            LayoutUnit collapsedBeforePos = max(marginInfo.positiveMargin(), childMargins.positiveMarginBefore());
+            LayoutUnit collapsedBeforeNeg = max(marginInfo.negativeMargin(), childMargins.negativeMarginBefore());
+            marginInfo.setMargin(collapsedBeforePos, collapsedBeforeNeg);
+            
+            // Now collapse the child's margins together, which means examining our
+            // bottom margin values as well. 
+            marginInfo.setPositiveMarginIfLarger(childMargins.positiveMarginAfter());
+            marginInfo.setNegativeMarginIfLarger(childMargins.negativeMarginAfter());
+
+            if (!marginInfo.canCollapseWithMarginBefore())
+                // We need to make sure that the position of the self-collapsing block
+                // is correct, since it could have overflowing content
+                // that needs to be positioned correctly (e.g., a block that
+                // had a specified height of 0 but that actually had subcontent).
+                logicalTop = logicalHeight() + collapsedBeforePos - collapsedBeforeNeg;
+        }
+    } else {
+        if (mustSeparateMarginBeforeForChild(child)) {
+            ASSERT(!marginInfo.discardMargin() || (marginInfo.discardMargin() && !marginInfo.margin()));
             setLogicalHeight(logicalHeight() + marginInfo.margin() + marginBeforeForChild(child));
             logicalTop = logicalHeight();
-        }
-        else if (!marginInfo.atBeforeSideOfBlock() ||
-            (!marginInfo.canCollapseMarginBeforeWithChildren()
-             && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginBeforeQuirk()))) {
+        } else if (!marginInfo.discardMargin() && (!marginInfo.atBeforeSideOfBlock()
+            || (!marginInfo.canCollapseMarginBeforeWithChildren()
+            && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginBeforeQuirk())))) {
             // We're collapsing with a previous sibling's margins and not
             // with the top of the block.
             setLogicalHeight(logicalHeight() + max(marginInfo.positiveMargin(), posTop) - max(marginInfo.negativeMargin(), negTop));
             logicalTop = logicalHeight();
         }
 
-        marginInfo.setPositiveMargin(childMargins.positiveMarginAfter());
-        marginInfo.setNegativeMargin(childMargins.negativeMarginAfter());
+        marginInfo.setDiscardMargin(childDiscardMarginAfter);
+        
+        if (!marginInfo.discardMargin()) {
+            marginInfo.setPositiveMargin(childMargins.positiveMarginAfter());
+            marginInfo.setNegativeMargin(childMargins.negativeMarginAfter());
+        } else
+            marginInfo.clearMargin();
 
         if (marginInfo.margin())
-            marginInfo.setMarginAfterQuirk(child->isMarginAfterQuirk() || style()->marginAfterCollapse() == MDISCARD);
+            marginInfo.setMarginAfterQuirk(child->isMarginAfterQuirk());
     }
     
     // If margins would pull us past the top of the next page, then we need to pull back and pretend like the margins
@@ -2079,12 +2106,19 @@ LayoutUnit RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& margin
         return yPos;
 
     if (child->isSelfCollapsingBlock()) {
+        bool childDiscardMargin = mustDiscardMarginBeforeForChild(child) || mustDiscardMarginAfterForChild(child);
+
         // For self-collapsing blocks that clear, they can still collapse their
         // margins with following siblings.  Reset the current margins to represent
         // the self-collapsing block's margins only.
-        MarginValues childMargins = marginValuesForChild(child);
-        marginInfo.setPositiveMargin(max(childMargins.positiveMarginBefore(), childMargins.positiveMarginAfter()));
-        marginInfo.setNegativeMargin(max(childMargins.negativeMarginBefore(), childMargins.negativeMarginAfter()));
+        // If DISCARD is specified for -webkit-margin-collapse, reset the margin values.
+        if (!childDiscardMargin) {
+            MarginValues childMargins = marginValuesForChild(child);
+            marginInfo.setPositiveMargin(max(childMargins.positiveMarginBefore(), childMargins.positiveMarginAfter()));
+            marginInfo.setNegativeMargin(max(childMargins.negativeMarginBefore(), childMargins.negativeMarginAfter()));
+        } else
+            marginInfo.clearMargin();
+        marginInfo.setDiscardMargin(childDiscardMargin);
 
         // CSS2.1 states:
         // "If the top and bottom margins of an element with clearance are adjoining, its margins collapse with 
@@ -2117,6 +2151,9 @@ LayoutUnit RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& margin
         // margins involved.
         setMaxMarginBeforeValues(oldTopPosMargin, oldTopNegMargin);
         marginInfo.setAtBeforeSideOfBlock(false);
+
+        // In case the child discarded the before margin of the block we need to reset the mustDiscardMarginBefore flag to the initial value.
+        setMustDiscardMarginBefore(style()->marginBeforeCollapse() == MDISCARD);
     }
 
     LayoutUnit logicalTop = yPos + heightIncrease;
@@ -2128,12 +2165,22 @@ LayoutUnit RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& margin
     return logicalTop;
 }
 
-void RenderBlock::marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const
+void RenderBlock::marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore, bool& discardMarginBefore) const
 {
-    // FIXME: We should deal with the margin-collapse-* style extensions that prevent collapsing and that discard margins.
     // Give up if in quirks mode and we're a body/table cell and the top margin of the child box is quirky.
-    if (document()->inQuirksMode() && child->isMarginBeforeQuirk() && (isTableCell() || isBody()))
+    // Give up if the child specified -webkit-margin-collapse: separate that prevents collapsing.
+    // FIXME: Use writing mode independent accessor for marginBeforeCollapse.
+    if ((document()->inQuirksMode() && child->isMarginBeforeQuirk() && (isTableCell() || isBody())) || child->style()->marginBeforeCollapse() == MSEPARATE)
+        return;
+
+    // The margins are discarded by a child that specified -webkit-margin-collapse: discard.
+    // FIXME: Use writing mode independent accessor for marginBeforeCollapse.
+    if (child->style()->marginBeforeCollapse() == MDISCARD) {
+        positiveMarginBefore = 0;
+        negativeMarginBefore = 0;
+        discardMarginBefore = true;
         return;
+    }
 
     LayoutUnit beforeChildMargin = marginBeforeForChild(child);
     positiveMarginBefore = max(positiveMarginBefore, beforeChildMargin);
@@ -2163,12 +2210,12 @@ void RenderBlock::marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& pos
     // Make sure to update the block margins now for the grandchild box so that we're looking at current values.
     if (grandchildBox->needsLayout()) {
         grandchildBox->computeAndSetBlockDirectionMargins(this);
-        grandchildBox->setMarginBeforeQuirk(grandchildBox->style()->marginBefore().quirk());
-        grandchildBox->setMarginAfterQuirk(grandchildBox->style()->marginAfter().quirk());
+        grandchildBox->setMarginBeforeQuirk(grandchildBox->style()->isMarginBeforeQuirk());
+        grandchildBox->setMarginAfterQuirk(grandchildBox->style()->isMarginAfterQuirk());
     }
 
     // Collapse the margin of the grandchild box with our own to produce an estimate.
-    childBlock->marginBeforeEstimateForChild(grandchildBox, positiveMarginBefore, negativeMarginBefore);
+    childBlock->marginBeforeEstimateForChild(grandchildBox, positiveMarginBefore, negativeMarginBefore, discardMarginBefore);
 }
 
 LayoutUnit RenderBlock::estimateLogicalTopPosition(RenderBox* child, const MarginInfo& marginInfo, LayoutUnit& estimateWithoutPagination)
@@ -2179,19 +2226,22 @@ LayoutUnit RenderBlock::estimateLogicalTopPosition(RenderBox* child, const Margi
     if (!marginInfo.canCollapseWithMarginBefore()) {
         LayoutUnit positiveMarginBefore = 0;
         LayoutUnit negativeMarginBefore = 0;
+        bool discardMarginBefore = false;
         if (child->selfNeedsLayout()) {
             // Try to do a basic estimation of how the collapse is going to go.
-            marginBeforeEstimateForChild(child, positiveMarginBefore, negativeMarginBefore);
+            marginBeforeEstimateForChild(child, positiveMarginBefore, negativeMarginBefore, discardMarginBefore);
         } else {
             // Use the cached collapsed margin values from a previous layout. Most of the time they
             // will be right.
             MarginValues marginValues = marginValuesForChild(child);
             positiveMarginBefore = max(positiveMarginBefore, marginValues.positiveMarginBefore());
             negativeMarginBefore = max(negativeMarginBefore, marginValues.negativeMarginBefore());
+            discardMarginBefore = mustDiscardMarginBeforeForChild(child);
         }
 
         // Collapse the result with our current margins.
-        logicalTopEstimate += max(marginInfo.positiveMargin(), positiveMarginBefore) - max(marginInfo.negativeMargin(), negativeMarginBefore);
+        if (!discardMarginBefore)
+            logicalTopEstimate += max(marginInfo.positiveMargin(), positiveMarginBefore) - max(marginInfo.negativeMargin(), negativeMarginBefore);
     }
 
     // Adjust logicalTopEstimate down to the next page if the margins are so large that we don't fit on the current
@@ -2266,6 +2316,13 @@ void RenderBlock::determineLogicalLeftPositionForChild(RenderBox* child)
 void RenderBlock::setCollapsedBottomMargin(const MarginInfo& marginInfo)
 {
     if (marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()) {
+        // Update the after side margin of the container to discard if the after margin of the last child also discards and we collapse with it.
+        // Don't update the max margin values because we won't need them anyway.
+        if (marginInfo.discardMargin()) {
+            setMustDiscardMarginAfter();
+            return;
+        }
+
         // Update our max pos/neg bottom margins, since we collapsed our bottom margins
         // with our children.
         setMaxMarginAfterValues(max(maxPositiveMarginAfter(), marginInfo.positiveMargin()), max(maxNegativeMarginAfter(), marginInfo.negativeMargin()));
@@ -2288,9 +2345,9 @@ void RenderBlock::handleAfterSideOfBlock(LayoutUnit beforeSide, LayoutUnit after
     // If we can't collapse with children then go ahead and add in the bottom margin.
     // Don't do this for ordinary anonymous blocks as only the enclosing box should add in
     // its margin.
-    if (!marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()
+    if (!marginInfo.discardMargin() && (!marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()
         && (!isAnonymousBlock() || isAnonymousColumnsBlock() || isAnonymousColumnSpanBlock())
-        && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginAfterQuirk()))
+        && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginAfterQuirk())))
         setLogicalHeight(logicalHeight() + marginInfo.margin());
         
     // Now add in our bottom border/padding.
@@ -2415,13 +2472,6 @@ void RenderBlock::layoutBlockChild(RenderBox* child, MarginInfo& marginInfo, Lay
     // The child is a normal flow object.  Compute the margins we will use for collapsing now.
     child->computeAndSetBlockDirectionMargins(this);
 
-    // Do not allow a collapse if the margin-before-collapse style is set to SEPARATE.
-    RenderStyle* childStyle = child->style();
-    if (childStyle->marginBeforeCollapse() == MSEPARATE) {
-        marginInfo.setAtBeforeSideOfBlock(false);
-        marginInfo.clearMargin();
-    }
-
     // Try to guess our correct logical top position.  In most cases this guess will
     // be correct.  Only if we're wrong (when we compute the real logical top position)
     // will we have to potentially relayout.
@@ -2515,7 +2565,7 @@ void RenderBlock::layoutBlockChild(RenderBox* child, MarginInfo& marginInfo, Lay
 
     // Update our height now that the child has been placed in the correct position.
     setLogicalHeight(logicalHeight() + logicalHeightForChild(child));
-    if (childStyle->marginAfterCollapse() == MSEPARATE) {
+    if (mustSeparateMarginAfterForChild(child)) {
         setLogicalHeight(logicalHeight() + marginAfterForChild(child));
         marginInfo.clearMargin();
     }
@@ -6819,6 +6869,99 @@ void RenderBlock::setMaxMarginAfterValues(LayoutUnit pos, LayoutUnit neg)
     m_rareData->m_margins.setNegativeMarginAfter(neg);
 }
 
+void RenderBlock::setMustDiscardMarginBefore(bool value)
+{
+    if (style()->marginBeforeCollapse() == MDISCARD) {
+        ASSERT(value);
+        return;
+    }
+    
+    if (!m_rareData && !value)
+        return;
+
+    if (!m_rareData)
+        m_rareData = adoptPtr(new RenderBlockRareData(this));
+
+    m_rareData->m_discardMarginBefore = value;
+}
+
+void RenderBlock::setMustDiscardMarginAfter(bool value)
+{
+    if (style()->marginAfterCollapse() == MDISCARD) {
+        ASSERT(value);
+        return;
+    }
+
+    if (!m_rareData && !value)
+        return;
+
+    if (!m_rareData)
+        m_rareData = adoptPtr(new RenderBlockRareData(this));
+
+    m_rareData->m_discardMarginAfter = value;
+}
+
+bool RenderBlock::mustDiscardMarginBefore() const
+{
+    return style()->marginBeforeCollapse() == MDISCARD || (m_rareData && m_rareData->m_discardMarginBefore);
+}
+
+bool RenderBlock::mustDiscardMarginAfter() const
+{
+    return style()->marginAfterCollapse() == MDISCARD || (m_rareData && m_rareData->m_discardMarginAfter);
+}
+
+bool RenderBlock::mustDiscardMarginBeforeForChild(const RenderBox* child) const
+{
+    ASSERT(!child->selfNeedsLayout());
+    if (!child->isWritingModeRoot())
+        return child->isRenderBlock() ? toRenderBlock(child)->mustDiscardMarginBefore() : (child->style()->marginBeforeCollapse() == MDISCARD);
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return child->isRenderBlock() ? toRenderBlock(child)->mustDiscardMarginAfter() : (child->style()->marginAfterCollapse() == MDISCARD);
+
+    // FIXME: We return false here because the implementation is not geometrically complete. We have values only for before/after, not start/end.
+    // In case the boxes are perpendicular we assume the property is not specified.
+    return false;
+}
+
+bool RenderBlock::mustDiscardMarginAfterForChild(const RenderBox* child) const
+{
+    ASSERT(!child->selfNeedsLayout());
+    if (!child->isWritingModeRoot())
+        return child->isRenderBlock() ? toRenderBlock(child)->mustDiscardMarginAfter() : (child->style()->marginAfterCollapse() == MDISCARD);
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return child->isRenderBlock() ? toRenderBlock(child)->mustDiscardMarginBefore() : (child->style()->marginBeforeCollapse() == MDISCARD);
+
+    // FIXME: See |mustDiscardMarginBeforeForChild| above.
+    return false;
+}
+
+bool RenderBlock::mustSeparateMarginBeforeForChild(const RenderBox* child) const
+{
+    ASSERT(!child->selfNeedsLayout());
+    const RenderStyle* childStyle = child->style();
+    if (!child->isWritingModeRoot())
+        return childStyle->marginBeforeCollapse() == MSEPARATE;
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return childStyle->marginAfterCollapse() == MSEPARATE;
+
+    // FIXME: See |mustDiscardMarginBeforeForChild| above.
+    return false;
+}
+
+bool RenderBlock::mustSeparateMarginAfterForChild(const RenderBox* child) const
+{
+    ASSERT(!child->selfNeedsLayout());
+    const RenderStyle* childStyle = child->style();
+    if (!child->isWritingModeRoot())
+        return childStyle->marginAfterCollapse() == MSEPARATE;
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return childStyle->marginBeforeCollapse() == MSEPARATE;
+
+    // FIXME: See |mustDiscardMarginBeforeForChild| above.
+    return false;
+}
+
 void RenderBlock::setPaginationStrut(LayoutUnit strut)
 {
     if (!m_rareData) {
index 9286fa48ed68c461bb7cd8c3177dbeff2779c1c5..e2d0bff97637f6638b8ee2dd9dc3bd429738709d 100644 (file)
@@ -455,12 +455,27 @@ protected:
     void setMaxMarginBeforeValues(LayoutUnit pos, LayoutUnit neg);
     void setMaxMarginAfterValues(LayoutUnit pos, LayoutUnit neg);
 
+    void setMustDiscardMarginBefore(bool = true);
+    void setMustDiscardMarginAfter(bool = true);
+
+    bool mustDiscardMarginBefore() const;
+    bool mustDiscardMarginAfter() const;
+
+    bool mustDiscardMarginBeforeForChild(const RenderBox*) const;
+    bool mustDiscardMarginAfterForChild(const RenderBox*) const;
+
+    bool mustSeparateMarginBeforeForChild(const RenderBox*) const;
+    bool mustSeparateMarginAfterForChild(const RenderBox*) const;
+
     void initMaxMarginValues()
     {
         if (m_rareData) {
             m_rareData->m_margins = MarginValues(RenderBlockRareData::positiveMarginBeforeDefault(this) , RenderBlockRareData::negativeMarginBeforeDefault(this),
                                                  RenderBlockRareData::positiveMarginAfterDefault(this), RenderBlockRareData::negativeMarginAfterDefault(this));
             m_rareData->m_paginationStrut = 0;
+
+            m_rareData->m_discardMarginBefore = false;
+            m_rareData->m_discardMarginAfter = false;
         }
     }
 
@@ -949,6 +964,8 @@ private:
         bool m_marginAfterQuirk : 1;
         bool m_determinedMarginBeforeQuirk : 1;
 
+        bool m_discardMargin : 1;
+
         // These flags track the previous maximal positive and negative margins.
         LayoutUnit m_positiveMargin;
         LayoutUnit m_negativeMargin;
@@ -966,33 +983,37 @@ private:
         void setMarginBeforeQuirk(bool b) { m_marginBeforeQuirk = b; }
         void setMarginAfterQuirk(bool b) { m_marginAfterQuirk = b; }
         void setDeterminedMarginBeforeQuirk(bool b) { m_determinedMarginBeforeQuirk = b; }
-        void setPositiveMargin(LayoutUnit p) { m_positiveMargin = p; }
-        void setNegativeMargin(LayoutUnit n) { m_negativeMargin = n; }
+        void setPositiveMargin(LayoutUnit p) { ASSERT(!m_discardMargin); m_positiveMargin = p; }
+        void setNegativeMargin(LayoutUnit n) { ASSERT(!m_discardMargin); m_negativeMargin = n; }
         void setPositiveMarginIfLarger(LayoutUnit p)
         {
+            ASSERT(!m_discardMargin);
             if (p > m_positiveMargin)
                 m_positiveMargin = p;
         }
         void setNegativeMarginIfLarger(LayoutUnit n)
         {
+            ASSERT(!m_discardMargin);
             if (n > m_negativeMargin)
                 m_negativeMargin = n;
         }
 
-        void setMargin(LayoutUnit p, LayoutUnit n) { m_positiveMargin = p; m_negativeMargin = n; }
+        void setMargin(LayoutUnit p, LayoutUnit n) { ASSERT(!m_discardMargin); m_positiveMargin = p; m_negativeMargin = n; }
+        void setCanCollapseMarginAfterWithChildren(bool collapse) { m_canCollapseMarginAfterWithChildren = collapse; }
+        void setDiscardMargin(bool value) { m_discardMargin = value; }
 
         bool atBeforeSideOfBlock() const { return m_atBeforeSideOfBlock; }
         bool canCollapseWithMarginBefore() const { return m_atBeforeSideOfBlock && m_canCollapseMarginBeforeWithChildren; }
         bool canCollapseWithMarginAfter() const { return m_atAfterSideOfBlock && m_canCollapseMarginAfterWithChildren; }
         bool canCollapseMarginBeforeWithChildren() const { return m_canCollapseMarginBeforeWithChildren; }
         bool canCollapseMarginAfterWithChildren() const { return m_canCollapseMarginAfterWithChildren; }
-        void setCanCollapseMarginAfterWithChildren(bool collapse) { m_canCollapseMarginAfterWithChildren = collapse; }
         bool quirkContainer() const { return m_quirkContainer; }
         bool determinedMarginBeforeQuirk() const { return m_determinedMarginBeforeQuirk; }
         bool marginBeforeQuirk() const { return m_marginBeforeQuirk; }
         bool marginAfterQuirk() const { return m_marginAfterQuirk; }
         LayoutUnit positiveMargin() const { return m_positiveMargin; }
         LayoutUnit negativeMargin() const { return m_negativeMargin; }
+        bool discardMargin() const { return m_discardMargin; }
         LayoutUnit margin() const { return m_positiveMargin - m_negativeMargin; }
     };
 
@@ -1010,7 +1031,7 @@ private:
     LayoutUnit collapseMargins(RenderBox* child, MarginInfo&);
     LayoutUnit clearFloatsIfNeeded(RenderBox* child, MarginInfo&, LayoutUnit oldTopPosMargin, LayoutUnit oldTopNegMargin, LayoutUnit yPos);
     LayoutUnit estimateLogicalTopPosition(RenderBox* child, const MarginInfo&, LayoutUnit& estimateWithoutPagination);
-    void marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const;
+    void marginBeforeEstimateForChild(RenderBox*, LayoutUnit&, LayoutUnit&, bool&) const;
     void determineLogicalLeftPositionForChild(RenderBox* child);
     void handleAfterSideOfBlock(LayoutUnit top, LayoutUnit bottom, MarginInfo&);
     void setCollapsedBottomMargin(const MarginInfo&);
@@ -1172,8 +1193,10 @@ protected:
             , m_paginationStrut(0)
             , m_pageLogicalOffset(0)
             , m_lineGridBox(0)
-            , m_shouldBreakAtLineToAvoidWidow(false)
             , m_lineBreakToAvoidWidow(0)
+            , m_shouldBreakAtLineToAvoidWidow(false)
+            , m_discardMarginBefore(false)
+            , m_discardMarginAfter(false)
         { 
         }
 
@@ -1181,7 +1204,6 @@ protected:
         { 
             return std::max<LayoutUnit>(block->marginBefore(), 0);
         }
-        
         static LayoutUnit negativeMarginBeforeDefault(const RenderBlock* block)
         { 
             return std::max<LayoutUnit>(-block->marginBefore(), 0);
@@ -1201,8 +1223,10 @@ protected:
         
         RootInlineBox* m_lineGridBox;
 
-        bool m_shouldBreakAtLineToAvoidWidow;
         RootInlineBox* m_lineBreakToAvoidWidow;
+        bool m_shouldBreakAtLineToAvoidWidow : 1;
+        bool m_discardMarginBefore : 1;
+        bool m_discardMarginAfter : 1;
      };
 
     OwnPtr<RenderBlockRareData> m_rareData;
index 5a5ed06c0228681606f61383bca7fe4d3a2f4d5b..ec57f90c13764ebb14e3d098c9d69245468fcab8 100644 (file)
@@ -377,6 +377,8 @@ public:
     bool hasBorder() const { return surround->border.hasBorder(); }
     bool hasPadding() const { return surround->padding.nonZero(); }
     bool hasOffset() const { return surround->offset.nonZero(); }
+    bool isMarginBeforeQuirk() const { return marginBefore().quirk(); }
+    bool isMarginAfterQuirk() const { return marginAfter().quirk(); }
 
     bool hasBackgroundImage() const { return m_background->background().hasImage(); }
     bool hasFixedBackgroundImage() const { return m_background->background().hasFixedImage(); }