summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwolfbeast <mcwerewolf@wolfbeast.com>2020-03-13 11:28:54 +0100
committerwolfbeast <mcwerewolf@wolfbeast.com>2020-04-14 13:06:11 +0200
commitb45f5c435eff41ac12d234f31432463e87c58bb9 (patch)
treebd10f9c87da9defb040fbd2ef44b2e11489f5e8b
parent3c73c0805cfb97689e9ce2419b7fe3625b3bc4a6 (diff)
downloadUXP-b45f5c435eff41ac12d234f31432463e87c58bb9.tar
UXP-b45f5c435eff41ac12d234f31432463e87c58bb9.tar.gz
UXP-b45f5c435eff41ac12d234f31432463e87c58bb9.tar.lz
UXP-b45f5c435eff41ac12d234f31432463e87c58bb9.tar.xz
UXP-b45f5c435eff41ac12d234f31432463e87c58bb9.zip
Issue #1485 - Fix incorrect grid cell sizing to min/max space.
There were actually two separate logical errors in this method: The first part is that "origSizes.isSome()" is simply a bogus requirement for applying min/max-sizes here. I'm still keeping the optimization of not needlessly copying the mSizes array (as originally intended) since it's a quite common case. The second bug is that min/max-sizes were only applied under the "if (fr != 0.0f)" block. This is bogus since the calculated 'fr' value depends on 'aAvailableSize' which might change by applying min/max-sizes and thus 'fr' could become non-zero in the second round. To fix, this patch just moves "applyMinMax" block out one level.
-rw-r--r--layout/generic/nsGridContainerFrame.cpp58
-rw-r--r--layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html24
-rw-r--r--layout/reftests/css-grid/grid-max-sizing-flex-007.html24
3 files changed, 77 insertions, 29 deletions
diff --git a/layout/generic/nsGridContainerFrame.cpp b/layout/generic/nsGridContainerFrame.cpp
index 959061e33..f771c9d7c 100644
--- a/layout/generic/nsGridContainerFrame.cpp
+++ b/layout/generic/nsGridContainerFrame.cpp
@@ -4822,14 +4822,14 @@ nsGridContainerFrame::Tracks::StretchFlexibleTracks(
: ri->ComputedMaxISize();
}
Maybe<nsTArray<TrackSize>> origSizes;
+ bool applyMinMax = (minSize != 0 || maxSize != NS_UNCONSTRAINEDSIZE) &&
+ aAvailableSize == NS_UNCONSTRAINEDSIZE;
// We iterate twice at most. The 2nd time if the grid size changed after
// applying a min/max-size (can only occur if aAvailableSize is indefinite).
while (true) {
float fr = FindUsedFlexFraction(aState, aGridItems, flexTracks,
aFunctions, aAvailableSize);
if (fr != 0.0f) {
- bool applyMinMax = (minSize != 0 || maxSize != NS_UNCONSTRAINEDSIZE) &&
- aAvailableSize == NS_UNCONSTRAINEDSIZE;
for (uint32_t i : flexTracks) {
float flexFactor = aFunctions.MaxSizingFor(i).GetFlexFractionValue();
nscoord flexLength = NSToCoordRound(flexFactor * fr);
@@ -4841,36 +4841,36 @@ nsGridContainerFrame::Tracks::StretchFlexibleTracks(
base = flexLength;
}
}
- if (applyMinMax && origSizes.isSome()) {
- // https://drafts.csswg.org/css-grid/#algo-flex-tracks
- // "If using this flex fraction would cause the grid to be smaller than
- // the grid container’s min-width/height (or larger than the grid
- // container’s max-width/height), then redo this step, treating the free
- // space as definite [...]"
- nscoord newSize = 0;
- for (auto& sz : mSizes) {
- newSize += sz.mBase;
- }
- const auto sumOfGridGaps = SumOfGridGaps();
- newSize += sumOfGridGaps;
- if (newSize > maxSize) {
- aAvailableSize = maxSize;
- } else if (newSize < minSize) {
- aAvailableSize = minSize;
- }
- if (aAvailableSize != NS_UNCONSTRAINEDSIZE) {
- // Reset min/max-size to ensure 'applyMinMax' becomes false next time.
- minSize = 0;
- maxSize = NS_UNCONSTRAINEDSIZE;
- aAvailableSize = std::max(0, aAvailableSize - sumOfGridGaps);
- // Restart with the original track sizes and definite aAvailableSize.
+ }
+ if (applyMinMax) {
+ applyMinMax = false;
+ // https://drafts.csswg.org/css-grid/#algo-flex-tracks
+ // "If using this flex fraction would cause the grid to be smaller than
+ // the grid container’s min-width/height (or larger than the grid
+ // container’s max-width/height), then redo this step, treating the free
+ // space as definite [...]"
+ nscoord newSize = 0;
+ for (auto& sz : mSizes) {
+ newSize += sz.mBase;
+ }
+ const auto sumOfGridGaps = SumOfGridGaps();
+ newSize += sumOfGridGaps;
+ if (newSize > maxSize) {
+ aAvailableSize = maxSize;
+ } else if (newSize < minSize) {
+ aAvailableSize = minSize;
+ }
+ if (aAvailableSize != NS_UNCONSTRAINEDSIZE) {
+ aAvailableSize = std::max(0, aAvailableSize - sumOfGridGaps);
+ // Restart with the original track sizes and definite aAvailableSize.
+ if (origSizes.isSome()) {
mSizes = Move(*origSizes);
origSizes.reset();
- if (aAvailableSize == 0) {
- break; // zero available size wouldn't change any sizes though...
- }
- continue;
+ } // else, no mSizes[].mBase were changed above so it's still correct
+ if (aAvailableSize == 0) {
+ break; // zero available size wouldn't change any sizes though...
}
+ continue;
}
}
break;
diff --git a/layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html b/layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html
index c5392d32c..b17a1cc02 100644
--- a/layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html
+++ b/layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html
@@ -107,4 +107,28 @@
<div class="item"></div>
</div>
+<pre>The first 6 grids should look the same:</pre>
+<div class="grid rows" style="grid: 1fr / 30px; height:83px">
+ <div class="item"></div>
+</div>
+<div class="grid rows" style="grid: 10px 1fr / 30px; height:83px">
+ <div class="item" style="grid-row:span 2"></div>
+</div>
+<div class="grid rows" style="grid: 1fr / 30px; height:83px">
+ <div class="item"></div>
+</div>
+<div class="grid rows" style="grid: 1fr 1fr / 30px; height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 1fr auto / 30px; height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 10px 1fr / 30px; height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 1fr 1fr / 30px; grid-row-gap:10px; height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:40px"></div></div>
+ <div class="item"><div style="height:40px"></div></div>
+</div>
+
</body></html>
diff --git a/layout/reftests/css-grid/grid-max-sizing-flex-007.html b/layout/reftests/css-grid/grid-max-sizing-flex-007.html
index ac9dcc77c..a2f39e95b 100644
--- a/layout/reftests/css-grid/grid-max-sizing-flex-007.html
+++ b/layout/reftests/css-grid/grid-max-sizing-flex-007.html
@@ -105,4 +105,28 @@
<div class="item"></div>
</div>
+<pre>The first 6 grids should look the same:</pre>
+<div class="grid rows" style="grid: 1fr / 30px; min-height:83px">
+ <div class="item"></div>
+</div>
+<div class="grid rows" style="grid: 10px 1fr / 30px; min-height:83px">
+ <div class="item" style="grid-row:span 2"></div>
+</div>
+<div class="grid rows" style="grid: 1fr / 30px; max-height:30px; min-height:83px">
+ <div class="item"></div>
+</div>
+<div class="grid rows" style="grid: 1fr 1fr / 30px; max-height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 1fr auto / 30px; max-height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 10px 1fr / 30px; max-height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:90px"></div></div>
+</div>
+<div class="grid rows" style="grid: 1fr 1fr / 30px; grid-row-gap:10px; max-height:83px">
+ <div class="item" style="grid-row:span 2"><div style="height:40px"></div></div>
+ <div class="item"><div style="height:40px"></div></div>
+</div>
+
</body></html>