[CSS Grid Layout] Grid items must set a new formatting context.
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Dec 2014 19:13:16 +0000 (19:13 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Dec 2014 19:13:16 +0000 (19:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139150

Reviewed by David Hyatt.

Source/WebCore:

Grid item's margins must not collapse even when they may be adjoining to
its content's margins. Also, setting a new formatting context prevents any
'float' protruding content on the adjoining grid items.

This patch also renames the expandsToEncloseOverhangingFloats to be more generic now,
determining whether a new formatting context is set or not. This affects not only to
how floats behave, but whether margins should collapse or not.

Tests: fast/css-grid-layout/float-not-protruding-into-next-grid-item.html
       fast/css-grid-layout/grid-item-margins-not-collapse.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::avoidsFloats): Using the new createsNewFormattingContext function.
(WebCore::RenderBlock::expandsToEncloseOverhangingFloats): Deleted.
* rendering/RenderBlock.h:
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::MarginInfo::MarginInfo): Using the new createsNewFormattingContext function.
(WebCore::RenderBlockFlow::rebuildFloatingObjectSetFromIntrudingFloats): Using the new createsNewFormattingContext function.
(WebCore::RenderBlockFlow::layoutBlock): Using the new createsNewFormattingContext function.
(WebCore::RenderBlockFlow::computeOverflow): Using the new createsNewFormattingContext function.
(WebCore::RenderBlockFlow::addOverhangingFloats): Using the new createsNewFormattingContext function.
(WebCore::RenderBlockFlow::needsLayoutAfterRegionRangeChange): Using the new createsNewFormattingContext function.
* rendering/RenderBox.cpp:
(WebCore::RenderBox::createsNewFormattingContext): Added.
(WebCore::RenderBox::avoidsFloats): Removed checks already defined in the new createsNewFormattingContext function.
* rendering/RenderBox.h:
(WebCore::RenderBox::isGridItem): Added.

LayoutTests:

Test to verify that grid items's margin don't collapese with its parent's margin
and there is no 'float' protruding content on the adjoining grid items.

I had to rebaseline the form-hides-table.html test because table-caption, which
is supposed to establish a new formatting context, does not allow margins collapsing.

* fast/css-grid-layout/float-not-protruding-into-next-grid-item-expected.html: Added.
* fast/css-grid-layout/float-not-protruding-into-next-grid-item.html: Added.
* fast/css-grid-layout/grid-item-margins-not-collapse-expected.html: Added.
* fast/css-grid-layout/grid-item-margins-not-collapse.html: Added.
* platform/gtk/fast/forms/form-hides-table-expected.txt: Rebaseline needed.
* platform/mac/fast/forms/form-hides-table-expected.txt: Rebaseline needed.
* platform/efl/TestExpectations: Mark fast/forms/form-hides-table-expected as failure.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item-expected.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse-expected.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse.html [new file with mode: 0644]
LayoutTests/platform/efl/TestExpectations
LayoutTests/platform/gtk/fast/forms/form-hides-table-expected.txt
LayoutTests/platform/mac/fast/forms/form-hides-table-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h

index 00865c7..3312415 100644 (file)
@@ -1,3 +1,24 @@
+2014-12-08  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Grid items must set a new formatting context.
+        https://bugs.webkit.org/show_bug.cgi?id=139150
+
+        Reviewed by David Hyatt.
+
+        Test to verify that grid items's margin don't collapese with its parent's margin
+        and there is no 'float' protruding content on the adjoining grid items.
+
+        I had to rebaseline the form-hides-table.html test because table-caption, which
+        is supposed to establish a new formatting context, does not allow margins collapsing.
+
+        * fast/css-grid-layout/float-not-protruding-into-next-grid-item-expected.html: Added.
+        * fast/css-grid-layout/float-not-protruding-into-next-grid-item.html: Added.
+        * fast/css-grid-layout/grid-item-margins-not-collapse-expected.html: Added.
+        * fast/css-grid-layout/grid-item-margins-not-collapse.html: Added.
+        * platform/gtk/fast/forms/form-hides-table-expected.txt: Rebaseline needed.
+        * platform/mac/fast/forms/form-hides-table-expected.txt: Rebaseline needed.
+        * platform/efl/TestExpectations: Mark fast/forms/form-hides-table-expected as failure.
+
 2014-12-08  Andrzej Badowski  <a.badowski@samsung.com>
 
         [EFL] Change expectations for two accessibility layout tests.
diff --git a/LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item-expected.html b/LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item-expected.html
new file mode 100644 (file)
index 0000000..a0ccbf0
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.cell {
+   width: 50px;
+   min-height: 50px
+}
+
+.invisibleFont {
+   color: lime;
+}
+
+.floatLeft {
+   float: left;
+}
+
+.clearLeft {
+   clear: left;
+}
+
+</style>
+</head>
+
+<body>
+
+<div>This test checks that grid item sets a new formatting context for its content, preventing any 'float' protruding content on the adjoining grid item ('Float' text shouldn't overflow the first row).</div>
+
+<div>
+  <div class="cell floatLeft firstRowFirstColumn">
+    <div class="text">Float</div>
+    <div class="text">Float</div>
+    <div class="text">Float</div>
+    <div class="text">Float</div>
+  </div>
+  <div class="cell floatLeft firstRowSecondColumn">
+    <div class="text invisibleFont">Float</div>
+    <div class="text invisibleFont">Float</div>
+    <div class="text invisibleFont">Float</div>
+    <div class="text invisibleFont">Float</div>
+  </div>
+  <div class="cell floatLeft clearLeft secondRowFirstColumn"></div>
+  <div class="cell floatLeft secondRowSecondColumn"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item.html b/LayoutTests/fast/css-grid-layout/float-not-protruding-into-next-grid-item.html
new file mode 100644 (file)
index 0000000..51fe493
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.grid {
+    -webkit-grid-auto-columns: 50px;
+    -webkit-grid-auto-rows: minmax(50px, -webkit-max-content);
+    width: -webkit-fit-content;
+}
+
+.cell {
+   width: 100%;
+   height: 100%;
+}
+
+.floatChild {
+   float: left;
+}
+
+</style>
+</head>
+
+<body>
+
+<div>This test checks that grid item sets a new formatting context for its content, preventing any 'float' protruding content on the adjoining grid item ('Float' text shouldn't overflow the first row).</div>
+
+<div class="grid">
+  <div class="cell firstRowFirstColumn">
+    <div class="floatChild">Float</div>
+    <div class="floatChild">Float</div>
+    <div class="floatChild">Float</div>
+    <div class="floatChild">Float</div>
+  </div>
+  <div class="cell firstRowSecondColumn"></div>
+  <div class="cell secondRowFirstColumn"></div>
+  <div class="cell secondRowSecondColumn"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse-expected.html b/LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse-expected.html
new file mode 100644 (file)
index 0000000..280998d
--- /dev/null
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+
+<div>This test checks that grid item's margins do not collapse with its content's margins (single margin in the first row and double between subsequent).</div>
+
+<div style="float: left">
+  <div><p margin="20px 0px">XXXXX</p></div>
+  <div style="float: left; margin:20px 0px;">XXXXX</div>
+  <div><p style="float: left;" margin="20px 0px">XXXXX</p></div>
+</div>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse.html b/LayoutTests/fast/css-grid-layout/grid-item-margins-not-collapse.html
new file mode 100644 (file)
index 0000000..16e2342
--- /dev/null
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+
+<div>This test checks that grid item's margins do not collapse with its content's margins (single margin in the first row and double between subsequent).</div>
+
+<div style="display: -webkit-grid;">
+  <div><p margin="20px 0px">XXXXX</p></div>
+  <div style="margin:20px 0px;">XXXXX</div>
+  <div><p margin="20px 0px">XXXXX</p></div>
+</div>
index fa66e63..1a1edb4 100644 (file)
@@ -2128,3 +2128,6 @@ webkit.org/b/136406 compositing/tiling/tiled-layer-resize.html [ Failure ]
 webkit.org/b/136406 compositing/video/video-reflection.html [ Failure ]
 webkit.org/b/136406 compositing/visibility/visibility-image-layers-dynamic.html [ Failure ]
 webkit.org/b/136406 compositing/visibility/visibility-simple-video-layer.html [ Failure ]
+
+# This test need a rebaseline.
+webkit.org/b/139150 fast/forms/form-hides-table.html [ Failure ]
\ No newline at end of file
index d84af39..27a32a8 100644 (file)
@@ -1,8 +1,8 @@
-layer at (0,0) size 785x624
+layer at (0,0) size 785x640
   RenderView at (0,0) size 785x600
-layer at (0,0) size 785x624
-  RenderBlock {HTML} at (0,0) size 785x624
-    RenderBody {BODY} at (8,8) size 769x608
+layer at (0,0) size 785x640
+  RenderBlock {HTML} at (0,0) size 785x640
+    RenderBody {BODY} at (8,8) size 769x624
       RenderBlock {P} at (0,0) size 769x17
         RenderText {#text} at (0,0) size 551x17
           text run at (0,0) width 551: "This page has a few tables within form elements within divs with various display styles."
@@ -155,9 +155,9 @@ layer at (0,0) size 785x624
                         RenderTableCell {TD} at (2,2) size 112x19 [r=0 c=0 rs=1 cs=1]
                           RenderText {#text} at (1,1) size 110x17
                             text run at (1,1) width 110: "display: table-cell"
-      RenderBlock {DIV} at (0,551) size 769x57
-        RenderTable at (0,0) size 55x57
-          RenderBlock {DIV} at (0,0) size 55x57
+      RenderBlock {DIV} at (0,551) size 769x73
+        RenderTable at (0,0) size 55x73
+          RenderBlock {DIV} at (0,0) size 55x73
             RenderBlock {FORM} at (0,0) size 55x57
               RenderTable {TABLE} at (0,0) size 55x57
                 RenderTableSection {TBODY} at (0,0) size 55x57
index 8092ab2..337eac0 100644 (file)
@@ -1,8 +1,8 @@
-layer at (0,0) size 785x642
+layer at (0,0) size 785x658
   RenderView at (0,0) size 785x600
-layer at (0,0) size 785x642
-  RenderBlock {HTML} at (0,0) size 785x642
-    RenderBody {BODY} at (8,8) size 769x626
+layer at (0,0) size 785x658
+  RenderBlock {HTML} at (0,0) size 785x658
+    RenderBody {BODY} at (8,8) size 769x642
       RenderBlock {P} at (0,0) size 769x18
         RenderText {#text} at (0,0) size 551x18
           text run at (0,0) width 551: "This page has a few tables within form elements within divs with various display styles."
@@ -155,9 +155,9 @@ layer at (0,0) size 785x642
                         RenderTableCell {TD} at (2,2) size 112x20 [r=0 c=0 rs=1 cs=1]
                           RenderText {#text} at (1,1) size 110x18
                             text run at (1,1) width 110: "display: table-cell"
-      RenderBlock {DIV} at (0,566) size 769x60
-        RenderTable at (0,0) size 55x60
-          RenderBlock {DIV} at (0,0) size 55x60
+      RenderBlock {DIV} at (0,566) size 769x76
+        RenderTable at (0,0) size 55x76
+          RenderBlock {DIV} at (0,0) size 55x76
             RenderBlock {FORM} at (0,0) size 55x60
               RenderTable {TABLE} at (0,0) size 55x60
                 RenderTableSection {TBODY} at (0,0) size 55x60
index 02250c7..dd9ab02 100644 (file)
@@ -1,3 +1,38 @@
+2014-12-08  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Grid items must set a new formatting context.
+        https://bugs.webkit.org/show_bug.cgi?id=139150
+
+        Reviewed by David Hyatt.
+
+        Grid item's margins must not collapse even when they may be adjoining to
+        its content's margins. Also, setting a new formatting context prevents any
+        'float' protruding content on the adjoining grid items.
+
+        This patch also renames the expandsToEncloseOverhangingFloats to be more generic now,
+        determining whether a new formatting context is set or not. This affects not only to
+        how floats behave, but whether margins should collapse or not.
+
+        Tests: fast/css-grid-layout/float-not-protruding-into-next-grid-item.html
+               fast/css-grid-layout/grid-item-margins-not-collapse.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::avoidsFloats): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlock::expandsToEncloseOverhangingFloats): Deleted.
+        * rendering/RenderBlock.h:
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::MarginInfo::MarginInfo): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlockFlow::rebuildFloatingObjectSetFromIntrudingFloats): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlockFlow::layoutBlock): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlockFlow::computeOverflow): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlockFlow::addOverhangingFloats): Using the new createsNewFormattingContext function.
+        (WebCore::RenderBlockFlow::needsLayoutAfterRegionRangeChange): Using the new createsNewFormattingContext function.
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::createsNewFormattingContext): Added.
+        (WebCore::RenderBox::avoidsFloats): Removed checks already defined in the new createsNewFormattingContext function.
+        * rendering/RenderBox.h:
+        (WebCore::RenderBox::isGridItem): Added.
+
 2014-12-08  Daniel Bates  <dabates@apple.com>
 
         [iOS] Attempt to fix the public SDK build after <https://trac.webkit.org/r176841>
index c2d2d8a..5f96814 100644 (file)
@@ -1092,12 +1092,6 @@ void RenderBlock::addVisualOverflowFromTheme()
         flowThread->addRegionsVisualOverflowFromTheme(this);
 }
 
-bool RenderBlock::expandsToEncloseOverhangingFloats() const
-{
-    return isInlineBlockOrInlineTable() || isFloatingOrOutOfFlowPositioned() || hasOverflowClip() || (parent() && parent()->isFlexibleBoxIncludingDeprecated())
-        || isTableCell() || isTableCaption() || isFieldset() || isWritingModeRoot() || isRoot() || isRenderFlowThread();
-}
-
 LayoutUnit RenderBlock::computeStartPositionDeltaForChildAvoidingFloats(const RenderBox& child, LayoutUnit childMarginStart, RenderRegion* region)
 {
     LayoutUnit startPosition = startOffsetForContent(region);
@@ -2381,8 +2375,6 @@ bool RenderBlock::avoidsFloats() const
 {
     // Floats can't intrude into our box if we have a non-auto column count or width.
     return RenderBox::avoidsFloats()
-        || !style().hasAutoColumnCount()
-        || !style().hasAutoColumnWidth()
         || style().hasFlowFrom();
 }
 
index 184dd2d..91a292f 100644 (file)
@@ -486,8 +486,6 @@ private:
     // FIXME-BLOCKFLOW: Remove virtualizaion when all callers have moved to RenderBlockFlow
     virtual VisiblePosition positionForPointWithInlineChildren(const LayoutPoint&, const RenderRegion*);
 
-    bool expandsToEncloseOverhangingFloats() const;
-
     RenderPtr<RenderBlock> clone() const;
     RenderBlock* continuationBefore(RenderObject* beforeChild);
 
index d5710cb..d60be47 100644 (file)
@@ -72,10 +72,7 @@ RenderBlockFlow::MarginInfo::MarginInfo(RenderBlockFlow& block, LayoutUnit befor
 {
     const RenderStyle& blockStyle = block.style();
     ASSERT(block.isRenderView() || block.parent());
-    m_canCollapseWithChildren = !block.isRenderView() && !block.isRoot() && !block.isOutOfFlowPositioned()
-        && !block.isFloating() && !block.isTableCell() && !block.hasOverflowClip() && !block.isInlineBlockOrInlineTable()
-        && !block.isRenderFlowThread() && !block.isWritingModeRoot() && !block.parent()->isFlexibleBox()
-        && blockStyle.hasAutoColumnCount() && blockStyle.hasAutoColumnWidth() && !blockStyle.columnSpan();
+    m_canCollapseWithChildren = !block.createsNewFormattingContext() && !block.isRenderView();
 
     m_canCollapseMarginBeforeWithChildren = m_canCollapseWithChildren && !beforeBorderPadding && blockStyle.marginBeforeCollapse() != MSEPARATE;
 
@@ -223,7 +220,7 @@ void RenderBlockFlow::rebuildFloatingObjectSetFromIntrudingFloats()
     RenderBlockFlow& parentBlock = downcast<RenderBlockFlow>(*parent());
     bool parentHasFloats = false;
     RenderObject* prev = previousSibling();
-    while (prev && (prev->isFloatingOrOutOfFlowPositioned() || !is<RenderBox>(*prev) || !is<RenderBlockFlow>(*prev) || downcast<RenderBlockFlow>(*prev).avoidsFloats())) {
+    while (prev && (!is<RenderBox>(*prev) || !is<RenderBlockFlow>(*prev) || downcast<RenderBlockFlow>(*prev).avoidsFloats() || downcast<RenderBlock>(prev)->createsNewFormattingContext())) {
         if (prev->isFloating())
             parentHasFloats = true;
         prev = prev->previousSibling();
@@ -474,7 +471,7 @@ void RenderBlockFlow::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalH
 
     // Expand our intrinsic height to encompass floats.
     LayoutUnit toAdd = borderAndPaddingAfter() + scrollbarLogicalHeight();
-    if (lowestFloatLogicalBottom() > (logicalHeight() - toAdd) && expandsToEncloseOverhangingFloats())
+    if (lowestFloatLogicalBottom() > (logicalHeight() - toAdd) && createsNewFormattingContext())
         setLogicalHeight(lowestFloatLogicalBottom() + toAdd);
     
     if (relayoutForPagination(statePusher) || relayoutToAvoidWidows(statePusher)) {
@@ -2079,7 +2076,7 @@ void RenderBlockFlow::computeOverflow(LayoutUnit oldClientAfterEdge, bool recomp
 {
     RenderBlock::computeOverflow(oldClientAfterEdge, recomputeFloats);
 
-    if (!multiColumnFlowThread() && (recomputeFloats || isRoot() || expandsToEncloseOverhangingFloats() || hasSelfPaintingLayer()))
+    if (!multiColumnFlowThread() && (recomputeFloats || createsNewFormattingContext() || hasSelfPaintingLayer()))
         addOverflowFromFloats();
 }
 
@@ -2587,7 +2584,7 @@ LayoutUnit RenderBlockFlow::lowestInitialLetterLogicalBottom() const
 LayoutUnit RenderBlockFlow::addOverhangingFloats(RenderBlockFlow& child, bool makeChildPaintOtherFloats)
 {
     // Prevent floats from being added to the canvas by the root element, e.g., <html>.
-    if (child.hasOverflowClip() || !child.containsFloats() || child.isRoot() || child.isWritingModeRoot() || child.isRenderFlowThread() || child.isRenderRegion())
+    if (!child.containsFloats() || child.createsNewFormattingContext())
         return 0;
 
     LayoutUnit childLogicalTop = child.logicalTop();
@@ -3077,7 +3074,7 @@ bool RenderBlockFlow::needsLayoutAfterRegionRangeChange() const
     // A block without floats or that expands to enclose them won't need a relayout
     // after a region range change. There is no overflow content needing relayout
     // in the region chain because the region range can only shrink after the estimation.
-    if (!containsFloats() || expandsToEncloseOverhangingFloats())
+    if (!containsFloats() || createsNewFormattingContext())
         return false;
 
     return true;
index e48eb9b..1c158d8 100644 (file)
@@ -4270,9 +4270,19 @@ bool RenderBox::shrinkToAvoidFloats() const
     return style().width().isAuto();
 }
 
+bool RenderBox::createsNewFormattingContext() const
+{
+    return isInlineBlockOrInlineTable() || isFloatingOrOutOfFlowPositioned() || hasOverflowClip() || isFlexItemIncludingDeprecated()
+        || isTableCell() || isTableCaption() || isFieldset() || isWritingModeRoot() || isRoot() || isRenderFlowThread() || isRenderRegion()
+#if ENABLE(CSS_GRID_LAYOUT)
+        || isGridItem()
+#endif
+        || style().specifiesColumns() || style().columnSpan();
+}
+
 bool RenderBox::avoidsFloats() const
 {
-    return isReplaced() || hasOverflowClip() || isHR() || isLegend() || isWritingModeRoot() || isFlexItemIncludingDeprecated();
+    return isReplaced() || isHR() || isLegend() || createsNewFormattingContext();
 }
 
 void RenderBox::addVisualEffectOverflow()
index 7ef0c27..1156056 100644 (file)
@@ -530,6 +530,10 @@ public:
     bool isDeprecatedFlexItem() const { return !isInline() && !isFloatingOrOutOfFlowPositioned() && parent() && parent()->isDeprecatedFlexibleBox(); }
     bool isFlexItemIncludingDeprecated() const { return !isInline() && !isFloatingOrOutOfFlowPositioned() && parent() && parent()->isFlexibleBoxIncludingDeprecated(); }
     
+#if ENABLE(CSS_GRID_LAYOUT)
+    bool isGridItem() const { return parent() && parent()->isRenderGrid(); }
+#endif
+
     virtual LayoutUnit lineHeight(bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const override;
     virtual int baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const override;
 
@@ -619,6 +623,8 @@ protected:
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) override;
     virtual void updateFromStyle() override;
 
+    bool createsNewFormattingContext() const;
+
     // Returns false if it could not cheaply compute the extent (e.g. fixed background), in which case the returned rect may be incorrect.
     bool getBackgroundPaintedExtent(LayoutRect&) const;
     virtual bool foregroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect, unsigned maxDepthToTest) const;