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.
This commit is contained in:
wolfbeast 2020-03-13 11:28:54 +01:00 committed by Roy Tam
commit 30e381eea2
3 changed files with 77 additions and 29 deletions

View file

@ -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 containers min-width/height (or larger than the grid
// containers 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 containers min-width/height (or larger than the grid
// containers 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;