diff options
author | wolfbeast <mcwerewolf@wolfbeast.com> | 2020-03-13 11:28:54 +0100 |
---|---|---|
committer | wolfbeast <mcwerewolf@wolfbeast.com> | 2020-03-13 11:28:54 +0100 |
commit | b3a4b8361ecd56ed762353f27dc86b3e28971f4b (patch) | |
tree | 8c54a764add8de5da050fd55db64139810347f6b | |
parent | 41e5925b4271c598601fdc77238bbf6497576594 (diff) | |
download | UXP-b3a4b8361ecd56ed762353f27dc86b3e28971f4b.tar UXP-b3a4b8361ecd56ed762353f27dc86b3e28971f4b.tar.gz UXP-b3a4b8361ecd56ed762353f27dc86b3e28971f4b.tar.lz UXP-b3a4b8361ecd56ed762353f27dc86b3e28971f4b.tar.xz UXP-b3a4b8361ecd56ed762353f27dc86b3e28971f4b.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.cpp | 58 | ||||
-rw-r--r-- | layout/reftests/css-grid/grid-max-sizing-flex-007-ref.html | 24 | ||||
-rw-r--r-- | layout/reftests/css-grid/grid-max-sizing-flex-007.html | 24 |
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> |