[css-grid] Grid containers reporting wrong preferred widths
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Aug 2015 10:54:07 +0000 (10:54 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Aug 2015 10:54:07 +0000 (10:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147486

Reviewed by Darin Adler.

Source/WebCore:

RenderGrid used to have its own overwritten version of
computePreferredLogicalWidths() because we didn't have an
implementation of computeIntrinsicLogicalWidths(). That
implementation was not as complete as RenderBlock's because it
was not taking into account min/max-width restrictions.

Provided that computeIntrinsicLogicalWidths() has been there
for some time we can safelly kill our overwrite and use
RenderBlock's version which addresses all the FIXMEs we had in
our code.

* rendering/RenderGrid.cpp:
* rendering/RenderGrid.h:

LayoutTests:

Added new test cases to check the preferred widths reported by
grid containers when they are sized under min/max-width
constraints.

* fast/css-grid-layout/grid-preferred-logical-widths-expected.txt:
* fast/css-grid-layout/grid-preferred-logical-widths.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-preferred-logical-widths-expected.txt
LayoutTests/fast/css-grid-layout/grid-preferred-logical-widths.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp
Source/WebCore/rendering/RenderGrid.h

index d496f94..3644802 100644 (file)
@@ -1,3 +1,17 @@
+2015-07-31  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-grid] Grid containers reporting wrong preferred widths
+        https://bugs.webkit.org/show_bug.cgi?id=147486
+
+        Reviewed by Darin Adler.
+
+        Added new test cases to check the preferred widths reported by
+        grid containers when they are sized under min/max-width
+        constraints.
+
+        * fast/css-grid-layout/grid-preferred-logical-widths-expected.txt:
+        * fast/css-grid-layout/grid-preferred-logical-widths.html:
+
 2015-08-31  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (r188820): fast/dom/HTMLObjectElement/object-as-frame.html is flaky
index 6c9d71b..24ae1a9 100644 (file)
@@ -60,3 +60,31 @@ PASS
 XX XX XX
 XX XX XX
 PASS
+XX XX XX
+XX XX XX
+PASS
+XX XX XX
+XX XX XX
+PASS
+XXXXX XXXXX
+XXXXX XXXXX
+PASS
+XXXXX XXXXX
+XXXXX XXXXX
+PASS
+XX XX XX
+XX XX XX
+PASS
+XX XX XX
+XX XX XX
+PASS
+PASS
+PASS
+XX XX XX
+XX XX XX
+PASS
+XX XX XX
+XX XX XX
+PASS
+PASS
+PASS
index 46b37d1..70d7e45 100644 (file)
@@ -4,22 +4,24 @@
 <link href="resources/grid.css" rel=stylesheet>
 <link href="../css-intrinsic-dimensions/resources/width-keyword-classes.css" rel=stylesheet>
 <style>
+
+.grid {
+    font: 10px/1 Ahem;
+}
+
 .gridMinContentFixed {
     -webkit-grid-template-columns: minmax(-webkit-min-content, 40px) minmax(-webkit-min-content, 40px);
     -webkit-grid-template-rows: 10px;
-    font: 10px/1 Ahem;
 }
 
 .gridFixedMinContent {
     -webkit-grid-template-columns: minmax(30px, -webkit-min-content) minmax(30px, -webkit-min-content);
     -webkit-grid-template-rows: 10px;
-    font: 10px/1 Ahem;
 }
 
 .gridFixedMaxContent {
     -webkit-grid-template-columns: minmax(40px, -webkit-max-content) minmax(40px, -webkit-max-content);
     -webkit-grid-template-rows: 10px;
-    font: 10px/1 Ahem;
 }
 
 .gridFixedFixed {
@@ -30,7 +32,6 @@
 .gridAutoContent {
     -webkit-grid-template-columns: auto auto;
     -webkit-grid-template-rows: 10px;
-    font: 10px/1 Ahem;
 }
 
 .gridFixedFraction {
 .margins {
     margin: 10px 20px 30px 40px;
 }
+
+.dummyContainer { }
+
+.minWidth70 {
+    min-width: 70px;
+}
+
+.maxWidth20 {
+    max-width: 20px;
+}
+
 </style>
 </head>
 <script src="../../resources/check-layout.js"></script>
-<body onload="checkLayout('.grid')">
+<body onload="checkLayout('.dummyContainer')">
 <body>
 <p>This test checks that the grid element's preferred logical widths are properly computed with different combinations of minmax().</p>
-<div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="40">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+<div class="dummyContainer">
+    <div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="40">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridMinContentFixed max-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridMinContentFixed max-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMinContent min-content" data-expected-height="10" data-expected-width="60">
+        <div class="firstRowFirstColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
 </div>
-<div class="grid gridFixedMinContent min-content" data-expected-height="10" data-expected-width="60">
-    <div class="firstRowFirstColumn">XXXXX XXXXX</div>
-    <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="100">
+        <div class="firstRowFirstColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
 </div>
-<div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="100">
-    <div class="firstRowFirstColumn">XXXXX XXXXX</div>
-    <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMaxContent min-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMaxContent min-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="160">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="160">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedFixed min-content" data-expected-height="10" data-expected-width="60"></div>
 </div>
-<div class="grid gridFixedFixed min-content" data-expected-height="10" data-expected-width="60"></div>
-<div class="grid gridFixedFixed max-content" data-expected-height="10" data-expected-width="80"></div>
-<div class="grid gridAutoContent min-content" data-expected-height="10" data-expected-width="40">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedFixed max-content" data-expected-height="10" data-expected-width="80"></div>
 </div>
-<div class="grid gridAutoContent max-content" data-expected-height="10" data-expected-width="160">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridAutoContent min-content" data-expected-height="10" data-expected-width="40">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
 
-<div class="grid gridFixedFraction min-content" data-expected-height="10" data-expected-width="10"></div>
-<div class="grid gridFixedFraction max-content" data-expected-height="10" data-expected-width="30"></div>
+<div class="dummyContainer">
+    <div class="grid gridAutoContent max-content" data-expected-height="10" data-expected-width="160">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedFraction min-content" data-expected-height="10" data-expected-width="10"></div>
+</div>
 
+<div class="dummyContainer">
+    <div class="grid gridFixedFraction max-content" data-expected-height="10" data-expected-width="30"></div>
+</div>
 <!-- Now with margin on one of the grid items. -->
-<div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="100">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn margins">XX XX XX</div>
+<div class="dummyContainer">
+    <div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="100">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn margins">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridMinContentFixed max-content" data-expected-height="10" data-expected-width="120">
-    <div class="firstRowFirstColumn margins">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridMinContentFixed max-content" data-expected-height="10" data-expected-width="120">
+        <div class="firstRowFirstColumn margins">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMinContent min-content" data-expected-height="10" data-expected-width="60">
-    <div class="firstRowFirstColumn">XXXXX XXXXX</div>
-    <div class="firstRowSecondColumn margins">XXXXX XXXXX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMinContent min-content" data-expected-height="10" data-expected-width="60">
+        <div class="firstRowFirstColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn margins">XXXXX XXXXX</div>
+    </div>
 </div>
-<div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="160">
-    <div class="firstRowFirstColumn margins">XXXXX XXXXX</div>
-    <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="160">
+        <div class="firstRowFirstColumn margins">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
 </div>
-<div class="grid gridFixedMaxContent min-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn margins">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMaxContent min-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn margins">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="220">
-    <div class="firstRowFirstColumn margins">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="220">
+        <div class="firstRowFirstColumn margins">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
 
 <!-- Spanning cells -->
-<div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="20">
-    <div class="firstRowBothColumn">XX XX XX</div>
+<div class="dummyContainer">
+    <div class="grid gridMinContentFixed min-content" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowBothColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowBothColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowBothColumn">XX XX XX</div>
+        <div class="firstRowBothColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridAutoContent min-content" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowBothColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer">
+    <div class="grid gridAutoContent max-content" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowBothColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<!-- Grids under min-width / max-width constraints -->
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="70">
+    <div class="grid gridMinContentFixed minWidth70" data-expected-height="10" data-expected-width="70">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridMinContentFixed maxWidth20" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="70">
+    <div class="grid gridFixedMinContent minWidth70" data-expected-height="10" data-expected-width="70">
+        <div class="firstRowFirstColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
+</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridFixedMinContent maxWidth20" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowFirstColumn">XXXXX XXXXX</div>
+        <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+    </div>
+</div>
+
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="80">
+    <div class="grid gridFixedMaxContent minWidth70" data-expected-height="10" data-expected-width="80">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMinContent max-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowBothColumn">XXXXX XXXXX</div>
-    <div class="firstRowSecondColumn">XXXXX XXXXX</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridFixedMaxContent maxWidth20" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
 </div>
-<div class="grid gridFixedMaxContent max-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowBothColumn">XX XX XX</div>
-    <div class="firstRowBothColumn">XX XX XX</div>
+
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="70">
+    <div class="grid gridFixedFixed minWidth70" data-expected-height="10" data-expected-width="70"></div>
 </div>
-<div class="grid gridAutoContent min-content" data-expected-height="10" data-expected-width="20">
-    <div class="firstRowFirstColumn">XX XX XX</div>
-    <div class="firstRowBothColumn">XX XX XX</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridFixedFixed maxWidth20" data-expected-height="10" data-expected-width="20"></div>
 </div>
-<div class="grid gridAutoContent max-content" data-expected-height="10" data-expected-width="80">
-    <div class="firstRowBothColumn">XX XX XX</div>
-    <div class="firstRowSecondColumn">XX XX XX</div>
+
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="70">
+    <div class="grid gridAutoContent minWidth70" data-expected-height="10" data-expected-width="70">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridAutoContent maxWidth20" data-expected-height="10" data-expected-width="20">
+        <div class="firstRowFirstColumn">XX XX XX</div>
+        <div class="firstRowSecondColumn">XX XX XX</div>
+    </div>
+</div>
+
+<div class="dummyContainer min-content" data-expected-height="10" data-expected-width="70">
+    <div class="grid gridFixedFraction minWidth70" data-expected-height="10" data-expected-width="70"></div>
+</div>
+
+<div class="dummyContainer max-content" data-expected-height="10" data-expected-width="20">
+    <div class="grid gridFixedFraction maxWidth20" data-expected-height="10" data-expected-width="20"></div>
 </div>
 
 </body>
index 05fbdb7..94490b5 100644 (file)
@@ -1,3 +1,24 @@
+2015-07-31  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-grid] Grid containers reporting wrong preferred widths
+        https://bugs.webkit.org/show_bug.cgi?id=147486
+
+        Reviewed by Darin Adler.
+
+        RenderGrid used to have its own overwritten version of
+        computePreferredLogicalWidths() because we didn't have an
+        implementation of computeIntrinsicLogicalWidths(). That
+        implementation was not as complete as RenderBlock's because it
+        was not taking into account min/max-width restrictions.
+
+        Provided that computeIntrinsicLogicalWidths() has been there
+        for some time we can safelly kill our overwrite and use
+        RenderBlock's version which addresses all the FIXMEs we had in
+        our code.
+
+        * rendering/RenderGrid.cpp:
+        * rendering/RenderGrid.h:
+
 2015-08-31  Sungmann Cho  <sungmann.cho@navercorp.com>
 
         Fix the WinCairo build after landing of webkit.org/b/148561.
index d3e85f1..36049d4 100644 (file)
@@ -311,25 +311,6 @@ void RenderGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo
         const_cast<RenderGrid*>(this)->clearGrid();
 }
 
-void RenderGrid::computePreferredLogicalWidths()
-{
-    ASSERT(preferredLogicalWidthsDirty());
-
-    m_minPreferredLogicalWidth = 0;
-    m_maxPreferredLogicalWidth = 0;
-
-    // FIXME: We don't take our own logical width into account. Once we do, we need to make sure
-    // we apply (and test the interaction with) min-width / max-width.
-
-    computeIntrinsicLogicalWidths(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
-
-    LayoutUnit borderAndPaddingInInlineDirection = borderAndPaddingLogicalWidth();
-    m_minPreferredLogicalWidth += borderAndPaddingInInlineDirection;
-    m_maxPreferredLogicalWidth += borderAndPaddingInInlineDirection;
-
-    setPreferredLogicalWidthsDirty(false);
-}
-
 void RenderGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection direction, GridSizingData& sizingData)
 {
     LayoutUnit availableLogicalSpace = (direction == ForColumns) ? availableLogicalWidth() : availableLogicalHeight(IncludeMarginBorderPadding);
index 2863fb8..6060284 100644 (file)
@@ -61,7 +61,6 @@ private:
     virtual const char* renderName() const override;
     virtual bool isRenderGrid() const override { return true; }
     virtual void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
-    virtual void computePreferredLogicalWidths() override;
 
     class GridIterator;
     class GridSizingData;