CSS 2.1 failure: margin-collapse-012 fails
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2012 18:16:58 +0000 (18:16 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2012 18:16:58 +0000 (18:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=80219

Reviewed by Eric Seidel.

Source/WebCore:

Tests: css2.1/20110323/margin-collapse-012.htm
       fast/css/margin-collapse-abspos-negmargin.htm

I also ran this against the full margin-collapse-* CSS 2.1 suite without regressions.

Per http://www.w3.org/TR/CSS21/box.html#collapsing-margins don't collapse the margins of
positioned blocks. Instead, just use the margin of the sibling/container to offset the
positioned block's logical top - its own margin gets added in later at
RenderBox::computePositionedLogicalHeightUsing().

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::adjustPositionedBlock):

LayoutTests:

* css2.1/20110323/margin-collapse-012.htm: Added.
* css2.1/20110323/margin-collapse-012-expected.html: Added.
* fast/css/margin-collapse-abspos-negmargin-expected.html: Added.
* fast/css/margin-collapse-abspos-negmargin.htm: Added.

* fast/block/positioning/auto/001.html:
* fast/block/positioning/auto/002.html:
* fast/block/positioning/auto/vertical-lr/002.html:
* fast/block/positioning/auto/vertical-rl/002.html:
* fast/dynamic/staticY.html:
  Remove the margins from these tests so the results remain the same. The new results
  are consistent with FF.

* platform/chromium-linux/fast/box-sizing/box-sizing-expected.png:
* platform/chromium-win/compositing/overflow/clip-descendents-expected.txt:
* platform/chromium-win/fast/box-sizing/box-sizing-expected.txt:
  I've rebaselined these - removing the margins doesn't seem helpful.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/css2.1/20110323/margin-collapse-012-expected.html [new file with mode: 0644]
LayoutTests/css2.1/20110323/margin-collapse-012.htm [new file with mode: 0644]
LayoutTests/fast/block/positioning/auto/001.html
LayoutTests/fast/block/positioning/auto/002.html
LayoutTests/fast/block/positioning/auto/vertical-lr/002.html
LayoutTests/fast/block/positioning/auto/vertical-rl/002.html
LayoutTests/fast/css/margin-collapse-abspos-negmargin-expected.html [new file with mode: 0644]
LayoutTests/fast/css/margin-collapse-abspos-negmargin.htm [new file with mode: 0644]
LayoutTests/fast/dynamic/staticY.html
LayoutTests/platform/chromium-linux/fast/box-sizing/box-sizing-expected.png
LayoutTests/platform/chromium-win/compositing/overflow/clip-descendents-expected.txt
LayoutTests/platform/chromium-win/fast/box-sizing/box-sizing-expected.txt
LayoutTests/platform/chromium/TestExpectations
LayoutTests/platform/efl/TestExpectations
LayoutTests/platform/gtk/TestExpectations
LayoutTests/platform/mac/TestExpectations
LayoutTests/platform/qt/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index f56447f..f881414 100644 (file)
@@ -1,3 +1,28 @@
+2012-07-31  Robert Hogan  <robert@webkit.org>
+
+        CSS 2.1 failure: margin-collapse-012 fails
+        https://bugs.webkit.org/show_bug.cgi?id=80219
+
+        Reviewed by Eric Seidel.
+
+        * css2.1/20110323/margin-collapse-012.htm: Added.
+        * css2.1/20110323/margin-collapse-012-expected.html: Added.
+        * fast/css/margin-collapse-abspos-negmargin-expected.html: Added.
+        * fast/css/margin-collapse-abspos-negmargin.htm: Added.
+
+        * fast/block/positioning/auto/001.html:
+        * fast/block/positioning/auto/002.html:
+        * fast/block/positioning/auto/vertical-lr/002.html:
+        * fast/block/positioning/auto/vertical-rl/002.html:
+        * fast/dynamic/staticY.html:
+          Remove the margins from these tests so the results remain the same. The new results
+          are consistent with FF.
+
+        * platform/chromium-linux/fast/box-sizing/box-sizing-expected.png:
+        * platform/chromium-win/compositing/overflow/clip-descendents-expected.txt:
+        * platform/chromium-win/fast/box-sizing/box-sizing-expected.txt:
+          I've rebaselined these - removing the margins doesn't seem helpful.
+
 2012-08-01  Xianzhu Wang  <wangxianzhu@chromium.org>
 
         Layout Test fast/text/descent-clip-in-scaled-page.html is failing on linux since it was added
diff --git a/LayoutTests/css2.1/20110323/margin-collapse-012-expected.html b/LayoutTests/css2.1/20110323/margin-collapse-012-expected.html
new file mode 100644 (file)
index 0000000..523f01a
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+    <head>
+        <title>CSS Test: Margin collapsing - absolute positioning and siblings</title>
+        <link rel="help" href="http://www.w3.org/TR/CSS21/box.html#collapsing-margins">
+        <meta name="flags" content="ahem image">
+        <meta name="assert" content="Absolutely positioned boxes do not collapse margins with siblings.">
+        <style type="text/css">
+            #div1
+            {
+                border-top: 1em solid green;
+                font: 20px/1em Ahem;
+                height: 2em;
+                width: 5em;
+                border-bottom: 1em solid green;
+            }
+        </style>
+    </head>
+    <body>
+        <p>Test passes if there is no red visible on the page.</p>
+        <div id="div1">
+        </div>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/css2.1/20110323/margin-collapse-012.htm b/LayoutTests/css2.1/20110323/margin-collapse-012.htm
new file mode 100644 (file)
index 0000000..1ccd1d0
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+    <head>
+        <title>CSS Test: Margin collapsing - absolute positioning and siblings</title>
+        <link rel="author" title="Microsoft" href="http://www.microsoft.com/">
+        <link rel="help" href="http://www.w3.org/TR/CSS21/box.html#collapsing-margins">
+        <meta name="flags" content="ahem image">
+        <meta name="assert" content="Absolutely positioned boxes do not collapse margins with siblings.">
+        <style type="text/css">
+            #div1
+            {
+                background: url('support/margin-collapse-2em-space.png') left -1em;
+                border-top: 1em solid green;
+                font: 20px/1em Ahem;
+                height: 3em;
+                width: 5em;
+            }
+            div div
+            {
+                height: 1em;
+                margin-top: 1em;
+                width: 5em;
+            }
+            #div2
+            {
+                height: 0;
+            }
+            #div3
+            {
+                background: green;
+                position: absolute;
+            }
+        </style>
+    </head>
+    <body>
+        <p>Test passes if there is no red visible on the page.</p>
+        <div id="div1">
+            <div id="div2"></div>
+            <div id="div3"></div>
+        </div>
+    </body>
+</html>
\ No newline at end of file
index c84d598..b908235 100644 (file)
@@ -7,7 +7,7 @@ should be 100x100, and there should be 50 pixels of padding on either side of th
 </p>
 <div style="position:absolute;width:100px;height:200px;border:20px solid black;padding:50px;">
 <div style="margin-bottom:50px"></div>
-<span style="position:absolute;margin-top:50px;width:100px;height:100px;background-color:green"></span>
+<span style="position:absolute;width:100px;height:100px;background-color:green"></span>
 <div style="position:absolute;width:100px;height:100px;background-color:olive;top:0px"></div>
 <div style="position:absolute;width:100px;height:100px;background-color:olive;top:200px"></div>
 </div>
index 8b33ceb..69f9330 100644 (file)
@@ -7,7 +7,7 @@ should be 100x100, and there should be 100 pixels of padding on the right side o
 </p>
 <div style="position:absolute;width:100px;height:200px;border:20px solid black;padding-top:50px;padding-bottom:50px;padding-left:px;padding-right:100px">
 <div style="margin-bottom:50px"></div>
-<div style="margin-left:100px;position:absolute;margin-top:50px;width:100px;height:100px;background-color:green"></div>
+<div style="margin-left:100px;position:absolute;width:100px;height:100px;background-color:green"></div>
 <div style="margin-left:100px;position:absolute;width:100px;height:100px;background-color:olive;top:0px"></div>
 <div style="margin-left:100px;position:absolute;width:100px;height:100px;background-color:olive;top:200px"></div>
 </div>
index 8a34b5b..7ec450f 100644 (file)
@@ -7,7 +7,7 @@ should be 100x100.
 </p>
 <div style="position:absolute;height:100px;width:200px;border:20px solid black;padding-left:50px;padding-right:50px;padding-bottom:100px">
 <div style="margin-right:50px"></div>
-<div style="margin-top:100px;position:absolute;margin-left:50px;width:100px;height:100px;background-color:green"></div>
+<div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:green"></div>
 <div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:olive;left:0px"></div>
 <div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:olive;left:200px"></div>
 </div>
index 9c89426..4868db5 100644 (file)
@@ -7,7 +7,7 @@ should be 100x100.
 </p>
 <div style="position:absolute;height:100px;width:200px;border:20px solid black;padding-left:50px;padding-right:50px;padding-bottom:100px">
 <div style="margin-left:50px"></div>
-<div style="margin-top:100px;position:absolute;margin-right:50px;width:100px;height:100px;background-color:green"></div>
+<div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:green"></div>
 <div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:olive;right:0px"></div>
 <div style="margin-top:100px;position:absolute;width:100px;height:100px;background-color:olive;right:200px"></div>
 </div>
diff --git a/LayoutTests/fast/css/margin-collapse-abspos-negmargin-expected.html b/LayoutTests/fast/css/margin-collapse-abspos-negmargin-expected.html
new file mode 100644 (file)
index 0000000..2754752
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+    <head>
+        <title>CSS Test: Margin collapsing - absolute positioning and siblings</title>
+        <link rel="help" href="http://www.w3.org/TR/CSS21/box.html#collapsing-margins">
+        <meta name="flags" content="ahem image">
+        <meta name="assert" content="Absolutely positioned boxes do not collapse margins with siblings.">
+        <style type="text/css">
+            #div1
+            {
+                border-top: 1em solid green;
+                font: 20px/1em Ahem;
+                height: 6em;
+                width: 5em;
+                border-bottom: 1em solid green;
+            }
+        </style>
+    </head>
+    <body>
+        <p>Test passes if there is no red visible on the page.</p>
+        <div id="div1">
+        </div>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/css/margin-collapse-abspos-negmargin.htm b/LayoutTests/fast/css/margin-collapse-abspos-negmargin.htm
new file mode 100644 (file)
index 0000000..f44a731
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+    <head>
+        <title>CSS Test: Margin collapsing - absolute positioning and siblings</title>
+        <link rel="help" href="http://www.w3.org/TR/CSS21/box.html#collapsing-margins">
+        <meta name="flags" content="ahem image">
+        <meta name="assert" content="Absolutely positioned boxes do not collapse margins with siblings.">
+        <style type="text/css">
+            #div1
+            {
+                border-top: 1em solid green;
+                font: 20px/1em Ahem;
+                height: 3em;
+                width: 5em;
+            }
+            div div
+            {
+                height: 1em;
+                width: 5em;
+            }
+            #div2
+            {
+                margin-bottom: -5em;
+            }
+            #div3
+            {
+                margin-top: 10em;
+                background: green;
+                position: absolute;
+            }
+            #div4
+            {
+                top: 12em;
+                height: 1em;
+                width: 5em;
+                background: red;
+                position: absolute;
+            }
+        </style>
+    </head>
+    <body>
+        <p>Test passes if there is no red visible on the page.</p>
+        <div id="div4"></div>
+        <div id="div1">
+            <div id="div2"></div>
+            <div id="div3"></div>
+        </div>
+    </body>
+</html>
\ No newline at end of file
index 20b0690..9ee4246 100644 (file)
@@ -52,7 +52,6 @@
     
         document.body.offsetTop;
     
-        t2.style.marginTop = "50px";
         t3.style.height = "50px";
         t4.style.height = "50px";
         t5.style.height = "50px";
index a202f98..45accd1 100644 (file)
Binary files a/LayoutTests/platform/chromium-linux/fast/box-sizing/box-sizing-expected.png and b/LayoutTests/platform/chromium-linux/fast/box-sizing/box-sizing-expected.png differ
index a03ca69..fd67b2c 100644 (file)
@@ -7,7 +7,7 @@ Test overflow clipping of composited elements in various configurations.
       (bounds 800.00 600.00)
       (children 6
         (GraphicsLayer
-          (position 48.00 66.00)
+          (position 48.00 82.00)
           (bounds 60.00 70.00)
           (children 1
             (GraphicsLayer
@@ -19,12 +19,12 @@ Test overflow clipping of composited elements in various configurations.
           )
         )
         (GraphicsLayer
-          (position 240.00 66.00)
+          (position 240.00 82.00)
           (bounds 60.00 70.00)
           (drawsContent 1)
         )
         (GraphicsLayer
-          (position 240.00 66.00)
+          (position 240.00 82.00)
           (bounds 60.00 70.00)
           (children 1
             (GraphicsLayer
@@ -35,7 +35,7 @@ Test overflow clipping of composited elements in various configurations.
           )
         )
         (GraphicsLayer
-          (position 240.00 66.00)
+          (position 240.00 82.00)
           (bounds 60.00 70.00)
           (children 1
             (GraphicsLayer
index d4fde2d..2e17986 100644 (file)
@@ -69,22 +69,22 @@ layer at (0,0) size 785x915
       RenderBlock (anonymous) at (0,849) size 769x41
         RenderBR {BR} at (120,0) size 0x19
       RenderBlock {HR} at (0,897) size 769x3 [border: (1px inset #000000)]
-layer at (13,418) size 20x20
-  RenderBlock (positioned) {DIV} at (13,417) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (43,418) size 20x20
-  RenderBlock (positioned) {DIV} at (43,417) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (73,418) size 20x20
-  RenderBlock (positioned) {DIV} at (73,417) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (103,418) size 20x20
-  RenderBlock (positioned) {DIV} at (103,417) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (13,428) size 20x20
+  RenderBlock (positioned) {DIV} at (13,427) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (43,428) size 20x20
+  RenderBlock (positioned) {DIV} at (43,427) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (73,428) size 20x20
+  RenderBlock (positioned) {DIV} at (73,427) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (103,428) size 20x20
+  RenderBlock (positioned) {DIV} at (103,427) size 20x21 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
     RenderBR {BR} at (6,6) size 0x19
     RenderBR {BR} at (6,26) size 0x19
     RenderBR {BR} at (6,46) size 0x19
-layer at (13,757) size 20x20
-  RenderImage {IMG} at (13,757) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (43,757) size 20x20
-  RenderImage {IMG} at (43,757) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (73,757) size 20x20
-  RenderImage {IMG} at (73,757) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
-layer at (103,757) size 20x20
-  RenderImage {IMG} at (103,757) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (13,767) size 20x20
+  RenderImage {IMG} at (13,767) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (43,767) size 20x20
+  RenderImage {IMG} at (43,767) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (73,767) size 20x20
+  RenderImage {IMG} at (73,767) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
+layer at (103,767) size 20x20
+  RenderImage {IMG} at (103,767) size 20x20 [color=#FFFFFF] [bgcolor=#FFA500] [border: (2px solid #000000)]
index 5554ad7..d72928f 100644 (file)
@@ -3503,3 +3503,7 @@ BUGWK92582 LINUX DEBUG : media/video-currentTime-set2.html = PASS CRASH
 
 // Started failing after http://trac.webkit.org/changeset/124314
 BUGWK92863 : fast/forms/input-widths.html = TEXT
+
+// Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=80219
+BUGWK80219 MAC WIN : fast/box-sizing/box-sizing.html = IMAGE+TEXT
+BUGWK80219 MAC : compositing/overflow/clip-descendents.html = TEXT
index 80456fc..f095181 100644 (file)
@@ -830,3 +830,7 @@ BUGWK92063 : fast/css/sticky/parsing-position-sticky.html = TEXT
 
 // This test depends on subpixel layout.
 BUGWK92352 : css3/flexbox/flex-rounding.html = TEXT
+
+// Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=80219
+BUGWK80219 : fast/box-sizing/box-sizing.html = IMAGE+TEXT
+BUGWK80219 : compositing/overflow/clip-descendents.html = TEXT
index bdf1775..2a39b65 100644 (file)
 // Tests that require new results.
 //////////////////////////////////////////////////////////////////////////////////////////
 
+// Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=80219
+BUGWK80219 : fast/box-sizing/box-sizing.html = IMAGE+TEXT
+BUGWK80219 : compositing/overflow/clip-descendents.html = TEXT
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // Expected failures
 //////////////////////////////////////////////////////////////////////////////////////////
index 0d11259..ae2365f 100644 (file)
@@ -304,4 +304,8 @@ BUGWK91505 : platform/mac/plugins/root-object-premature-delete-crash.html = CRAS
 BUGWK91552 : fast/text/descent-clip-in-scaled-page.html = IMAGE
 
 // Layout Test css3/filters/custom/custom-filter-animation.html is failing
-BUGWK84813 : css3/filters/custom/custom-filter-animation.html = TEXT
\ No newline at end of file
+BUGWK84813 : css3/filters/custom/custom-filter-animation.html = TEXT
+
+// Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=80219
+BUGWK80219 : fast/box-sizing/box-sizing.html = IMAGE+TEXT
+BUGWK80219 : compositing/overflow/clip-descendents.html = TEXT
index 808fcf9..5985333 100644 (file)
@@ -124,3 +124,7 @@ BUGWK91379 : http/tests/security/contentSecurityPolicy/policy-does-not-affect-ch
 BUGWK91379 : http/tests/security/contentSecurityPolicy/object-src-none-allowed.html = TEXT PASS
 
 BUGWK85811 DEBUG : fast/events/message-port-close.html = CRASH
+
+// Needs rebaseline after https://bugs.webkit.org/show_bug.cgi?id=80219
+BUGWK80219 : fast/box-sizing/box-sizing.html = IMAGE+TEXT
+BUGWK80219 : compositing/overflow/clip-descendents.html = TEXT
index 716fedd..6d35654 100644 (file)
@@ -1,3 +1,23 @@
+2012-07-31  Robert Hogan  <robert@webkit.org>
+
+        CSS 2.1 failure: margin-collapse-012 fails
+        https://bugs.webkit.org/show_bug.cgi?id=80219
+
+        Reviewed by Eric Seidel.
+
+        Tests: css2.1/20110323/margin-collapse-012.htm
+               fast/css/margin-collapse-abspos-negmargin.htm
+
+        I also ran this against the full margin-collapse-* CSS 2.1 suite without regressions.
+
+        Per http://www.w3.org/TR/CSS21/box.html#collapsing-margins don't collapse the margins of 
+        positioned blocks. Instead, just use the margin of the sibling/container to offset the 
+        positioned block's logical top - its own margin gets added in later at 
+        RenderBox::computePositionedLogicalHeightUsing().
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::adjustPositionedBlock):
+
 2012-08-01  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r124334.
index 7e64014..56c2b24 100755 (executable)
@@ -1691,18 +1691,11 @@ void RenderBlock::adjustPositionedBlock(RenderBox* child, const MarginInfo& marg
     setStaticInlinePositionForChild(child, logicalTop, startOffsetForContent(logicalTop));
 
     if (!marginInfo.canCollapseWithMarginBefore()) {
-        child->computeBlockDirectionMargins(this);
-        LayoutUnit marginBefore = marginBeforeForChild(child);
+        // Positioned blocks don't collapse margins, so add the margin provided by
+        // the container now. The child's own margin is added later when calculating its logical top.
         LayoutUnit collapsedBeforePos = marginInfo.positiveMargin();
         LayoutUnit collapsedBeforeNeg = marginInfo.negativeMargin();
-        if (marginBefore > 0) {
-            if (marginBefore > collapsedBeforePos)
-                collapsedBeforePos = marginBefore;
-        } else {
-            if (-marginBefore > collapsedBeforeNeg)
-                collapsedBeforeNeg = -marginBefore;
-        }
-        logicalTop += (collapsedBeforePos - collapsedBeforeNeg) - marginBefore;
+        logicalTop += collapsedBeforePos - collapsedBeforeNeg;
     }
     
     RenderLayer* childLayer = child->layer();