- Revision
- 225741
- Author
- r...@igalia.com
- Date
- 2017-12-11 01:11:19 -0800 (Mon, 11 Dec 2017)
Log Message
REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
https://bugs.webkit.org/show_bug.cgi?id=180287
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
* web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt:
Update expectations as the test is now passing.
Source/WebCore:
In r221931 we moved the stretch phase as the last step of
the track sizing algorithm.
However this introduced a regression as we were no longer
taking into account the grid container min-width|height constraints
during this step.
The CSS WG modified the spec so it now defines what to do
in these situations (https://drafts.csswg.org/css-grid/#algo-stretch):
"If the free space is indefinite, but the grid container
has a definite min-width/height, use that size to calculate
the free space for this step instead."
This patch adds a new method
GridTrackSizingAlgorithmStrategy::freeSpaceForStretchAutoTracksStep().
When we're in the DefiniteSizeStrategy it just returns the current
free space.
For the IndefiniteSizeStrategy in the columns case we don't need
any special computation (the same that happens in
recomputeUsedFlexFractionIfNeeded()); for rows it uses the min size
of the grid container (respecting min-width|height properties)
to calculate the free space.
Test: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html
* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
(WebCore::DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
(WebCore::GridTrackSizingAlgorithm::stretchAutoTracks):
* rendering/GridTrackSizingAlgorithm.h:
LayoutTests:
* TestExpectations: Now layout-algorithm/grid-stretch-respects-min-size-001.html
from WPT is passing, so this patch removes it from TestExpectations file.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225740 => 225741)
--- trunk/LayoutTests/ChangeLog 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/LayoutTests/ChangeLog 2017-12-11 09:11:19 UTC (rev 225741)
@@ -1,3 +1,13 @@
+2017-12-11 Manuel Rego Casasnovas <r...@igalia.com>
+
+ REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
+ https://bugs.webkit.org/show_bug.cgi?id=180287
+
+ Reviewed by Darin Adler.
+
+ * TestExpectations: Now layout-algorithm/grid-stretch-respects-min-size-001.html
+ from WPT is passing, so this patch removes it from TestExpectations file.
+
2017-12-10 Minsheng Liu <lam...@liu.ms>
Incorrect bounds inside <mover>/<munder> when a stretchy operator is present
Modified: trunk/LayoutTests/TestExpectations (225740 => 225741)
--- trunk/LayoutTests/TestExpectations 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/LayoutTests/TestExpectations 2017-12-11 09:11:19 UTC (rev 225741)
@@ -479,7 +479,6 @@
webkit.org/b/165062 fast/css-grid-layout/grid-baseline-margins.html [ ImageOnlyFailure ]
webkit.org/b/169271 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-sizing-alignment-001.html [ ImageOnlyFailure ]
webkit.org/b/180283 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html [ ImageOnlyFailure ]
-webkit.org/b/180287 imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html [ ImageOnlyFailure ]
webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-001.html [ ImageOnlyFailure ]
webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-002.html [ ImageOnlyFailure ]
webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-003.html [ ImageOnlyFailure ]
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (225740 => 225741)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2017-12-11 09:11:19 UTC (rev 225741)
@@ -1,3 +1,13 @@
+2017-12-11 Manuel Rego Casasnovas <r...@igalia.com>
+
+ REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
+ https://bugs.webkit.org/show_bug.cgi?id=180287
+
+ Reviewed by Darin Adler.
+
+ * web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt:
+ Update expectations as the test is now passing.
+
2017-12-08 Chris Dumez <cdu...@apple.com>
ServiceWorkerGlobalScope is a global object and should be marked as [ImplicitThis] in the IDL
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt (225740 => 225741)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt 2017-12-11 09:11:19 UTC (rev 225741)
@@ -59,17 +59,9 @@
<div class="item" data-expected-width="200" data-expected-height="56"></div>
</div>
height expected 56 but got 0
-FAIL .grid 13 assert_equals:
-<div class="grid wholeWidth" style="min-height: 100px; bottom: 0; box-sizing: border-box;" data-expected-width="200" data-expected-height="100">
- <div class="item" data-expected-width="154" data-expected-height="56"></div>
- </div>
-height expected 56 but got 0
+PASS .grid 13
PASS .grid 14
-FAIL .grid 15 assert_equals:
-<div class="grid wholeWidth" style="min-height: 100px; bottom: 0;" data-expected-width="200" data-expected-height="144">
- <div class="item" data-expected-width="154" data-expected-height="100"></div>
- </div>
-height expected 100 but got 0
+PASS .grid 15
PASS .grid 16
FAIL .grid 17 assert_equals:
<div class="grid wholeWidth" style="bottom: 0; box-sizing: border-box;" data-expected-width="200" data-expected-height="100">
@@ -91,16 +83,8 @@
<div class="item" data-expected-width="154" data-expected-height="56"></div>
</div>
height expected 56 but got 0
-FAIL .grid 21 assert_equals:
-<div class="grid wholeWidth" style="min-height: 100px; box-sizing: border-box;" data-expected-width="200" data-expected-height="100">
- <div class="item" data-expected-width="154" data-expected-height="56"></div>
- </div>
-height expected 56 but got 0
+PASS .grid 21
PASS .grid 22
-FAIL .grid 23 assert_equals:
-<div class="grid wholeWidth" style="min-height: 100px;" data-expected-width="200" data-expected-height="144">
- <div class="item" data-expected-width="154" data-expected-height="100"></div>
- </div>
-height expected 100 but got 0
+PASS .grid 23
PASS .grid 24
Modified: trunk/Source/WebCore/ChangeLog (225740 => 225741)
--- trunk/Source/WebCore/ChangeLog 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/Source/WebCore/ChangeLog 2017-12-11 09:11:19 UTC (rev 225741)
@@ -1,3 +1,40 @@
+2017-12-11 Manuel Rego Casasnovas <r...@igalia.com>
+
+ REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
+ https://bugs.webkit.org/show_bug.cgi?id=180287
+
+ Reviewed by Darin Adler.
+
+ In r221931 we moved the stretch phase as the last step of
+ the track sizing algorithm.
+ However this introduced a regression as we were no longer
+ taking into account the grid container min-width|height constraints
+ during this step.
+
+ The CSS WG modified the spec so it now defines what to do
+ in these situations (https://drafts.csswg.org/css-grid/#algo-stretch):
+ "If the free space is indefinite, but the grid container
+ has a definite min-width/height, use that size to calculate
+ the free space for this step instead."
+
+ This patch adds a new method
+ GridTrackSizingAlgorithmStrategy::freeSpaceForStretchAutoTracksStep().
+ When we're in the DefiniteSizeStrategy it just returns the current
+ free space.
+ For the IndefiniteSizeStrategy in the columns case we don't need
+ any special computation (the same that happens in
+ recomputeUsedFlexFractionIfNeeded()); for rows it uses the min size
+ of the grid container (respecting min-width|height properties)
+ to calculate the free space.
+
+ Test: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html
+
+ * rendering/GridTrackSizingAlgorithm.cpp:
+ (WebCore::IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
+ (WebCore::DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
+ (WebCore::GridTrackSizingAlgorithm::stretchAutoTracks):
+ * rendering/GridTrackSizingAlgorithm.h:
+
2017-12-10 Minsheng Liu <lam...@liu.ms>
Incorrect bounds inside <mover>/<munder> when a stretchy operator is present
Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp (225740 => 225741)
--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp 2017-12-11 09:11:19 UTC (rev 225741)
@@ -789,6 +789,7 @@
void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
+ LayoutUnit freeSpaceForStretchAutoTracksStep() const override;
};
LayoutUnit IndefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
@@ -886,8 +887,19 @@
void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
+ LayoutUnit freeSpaceForStretchAutoTracksStep() const override;
};
+LayoutUnit IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep() const
+{
+ ASSERT(!m_algorithm.freeSpace(direction()));
+ if (direction() == ForColumns)
+ return LayoutUnit();
+
+ auto minSize = renderGrid()->computeContentLogicalHeight(MinSize, renderGrid()->style().logicalMinHeight(), std::nullopt);
+ return minSize.value() - computeTrackBasedSize();
+}
+
LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
{
LayoutUnit marginLogicalWidth =
@@ -926,6 +938,11 @@
return findFrUnitSize(allTracksSpan, freeSpace.value());
}
+LayoutUnit DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep() const
+{
+ return m_algorithm.freeSpace(direction()).value();
+}
+
bool DefiniteSizeStrategy::recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const
{
UNUSED_PARAM(flexFraction);
@@ -1040,16 +1057,14 @@
void GridTrackSizingAlgorithm::stretchAutoTracks()
{
- auto currentFreeSpace = freeSpace(m_direction);
- if (m_autoSizedTracksForStretchIndex.isEmpty()
- || !currentFreeSpace
- || currentFreeSpace.value() <= 0
+ auto currentFreeSpace = m_strategy->freeSpaceForStretchAutoTracksStep();
+ if (m_autoSizedTracksForStretchIndex.isEmpty() || currentFreeSpace <= 0
|| (m_renderGrid->contentAlignment(m_direction).distribution() != ContentDistributionStretch))
return;
Vector<GridTrack>& allTracks = tracks(m_direction);
unsigned numberOfAutoSizedTracks = m_autoSizedTracksForStretchIndex.size();
- LayoutUnit sizeToIncrease = currentFreeSpace.value() / numberOfAutoSizedTracks;
+ LayoutUnit sizeToIncrease = currentFreeSpace / numberOfAutoSizedTracks;
for (const auto& trackIndex : m_autoSizedTracksForStretchIndex) {
auto& track = allTracks[trackIndex];
track.setBaseSize(track.baseSize() + sizeToIncrease);
Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h (225740 => 225741)
--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h 2017-12-11 08:31:38 UTC (rev 225740)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h 2017-12-11 09:11:19 UTC (rev 225741)
@@ -223,6 +223,7 @@
virtual void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) = 0;
virtual double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> initialFreeSpace) const = 0;
virtual bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const = 0;
+ virtual LayoutUnit freeSpaceForStretchAutoTracksStep() const = 0;
protected:
GridTrackSizingAlgorithmStrategy(GridTrackSizingAlgorithm& algorithm)
