This bug is regressed by Bug 1686603 Part 4 [1]. When the used flex-basis is
'content', the old code computes flex base size by using 'auto' as the main size
in `nsContainerFrame::ComputeSizeWithIntrinsicDimensions()`. The method is for
replaced elements to compute sizes, even if the element has no preferred
aspect-ratio such as an `<svg>` without viewBox nor aspect-ratio property.
However, Bug 1686603 Part 4 made replaced elements without preferred
aspect-ratio uses 'max-content' when computing flex base size. Unfortunately, we
only trigger the replaced elements intrinsic sizing via 'auto' but not via
'max-content', so this patch restores the behavior via emplacing 'auto' in
`styleFlexBaseSize`.
[1] https://phabricator.services.mozilla.com/D101795
Differential Revision: https://phabricator.services.mozilla.com/D155628
In the description of the mTruncated bit, its purpose is the same as calling
SetInlineLineBreakBeforeAndReset(). We've removed all its usages in previous
patches, so the bit is no longer needed.
Differential Revision: https://phabricator.services.mozilla.com/D151461
Note this patch only prevents "break-inside:avoid" flex containers from being
broken when they are not at the top of their current page/column. When they are
at the top of their current page/column, this patch fails to help since
`nsContainerFrame::ShouldAvoidBreakInside()` returns `false` via the
`mIsTopOfPage` check. This is essentially bug 793686.
Differential Revision: https://phabricator.services.mozilla.com/D149318
This patch shouldn't change the behavior.
* It's OK to get the item's offset only when its relatively positioned, because
`ApplyRelativePositioning` doesn't use the offset parameter if the frame is
sticky positioned. Document `ApplyRelativePositioning`'s parameters to clarify
this. Also, we don't need to call apply relative positioning to a
non-relatively-and-non-sticky-positioned item.
* Print `FLEX_LOG` after applying relative-positioning because `aFramePos` will
be adjusted in `ApplyRelativePositioning`, and we really want to see the
item's "final position".
* `aReflowInput` parameter is removed because it's only used to get flex
container's WritingMode. We can get it from FlexItem.
Differential Revision: https://phabricator.services.mozilla.com/D140703
`nsIFrame::IsRelativelyPositioned()` tests "position:relative" or
"position:sticky", but its naming is misleading until you've dug into the
underlying helper `nsStyleDisplay::IsRelativelyPositionedStyle()`.
Differential Revision: https://phabricator.services.mozilla.com/D140273
Currently, we pass all the five fields in FlexLayoutResult separately into
ReflowChildren(), but we really should just pass FlexLayoutResult instead.
Differential Revision: https://phabricator.services.mozilla.com/D138100
We've got the tentative cross size before calling DoFlexLayout() in Reflow(), so
we can just use that value in DoFlexLayout and a few other methods.
Also, add "ContentBox" to naming of the main size argument ComputeMainSize().
Differential Revision: https://phabricator.services.mozilla.com/D137365
I found in/out parameters confusing when reasoning the data flow. Aggregating
DoFlexLayout's output data in a struct also reduces DoFlexLayout's number of
arguments.
Differential Revision: https://phabricator.services.mozilla.com/D137364
The `struts` array is used only within DoFlexLayout, so we should move it into
`if (!GetPrevInFlow()) { ... }` branch.
Also, move `nsTArray<StrutInfo>&` argument on DoFlexLayout() to the second to
last place so that the output arguments are grouped together after applying Part
4.
Differential Revision: https://phabricator.services.mozilla.com/D137363
Move the assertion for unconstrained isize to the beginning of Reflow() because
we check it in all cases -- in GetMainSizeFromReflowInput when a flex container
is row-oriented, or in the old code computing gap size and ComputeCrossSize()
when a flex container is column-oriented.
Differential Revision: https://phabricator.services.mozilla.com/D137361
We introduced this comment at the very beginning of the flexbox implementation.
Since then, WritingModes.h has obtained the utilities dealing with logical axis,
and CSSAlignUtils.h has supported CSS alignment for both flexbox and grid. All
the helper functions remaining in nsFlexContainerFrame.cpp are related to
flexbox, so I feel it's time we retire this comment.
NPOTB DONTBUILD because this patch changes only comments.
Differential Revision: https://phabricator.services.mozilla.com/D134652
In Flexbox spec 4.1, Example 3 [1]:
... since the absolutely-positioned box is considered to be
"fixed-size", a value of stretch is treated the same as flex-start.
Also, per Alignment 3 spec [2]:
The default fallback alignment for 'stretch' is 'flex-start'.
Thus, when computing the alignment for flexbox's abspos children in
CSSAlignmentForAbsPosChild(), we convert 'stretch' to 'flex-start', and let the
subsequent logic convert 'flex-start' to either 'start' or 'end', because
nsAbsoluteContainingBlock don't know how to deal with the flex-relative axis.
This patch makes us behave the same as Google Chrome on the modified testcases.
[1] https://drafts.csswg.org/css-flexbox/#abspos-items
[2] https://drafts.csswg.org/css-align/#valdef-align-content-stretch
Differential Revision: https://phabricator.services.mozilla.com/D134543
Currently, we use IsPhysicalLTR() to query which is the start side of the
physical left <-> right axis. However, for vertical row-oriented flex
containers, physical left <-> right axis is its cross axis. We really should
query IsBidiLTR() instead because its main axis is parallel to line-left <->
line-right axis (i.e. inline axis).
flexbox-justify-content-wmvert-003.html is adapted from
flexbox-justify-content-wmvert-002.html.
Differential Revision: https://phabricator.services.mozilla.com/D133168
This patch clamps the `resolvedMinSize` before the size is passed into
`UpdateMainMinSize`, which is where the assertion is triggered.
Also, tweak the MOZ_ASSERT a bit because it's best to test an nscoord is
not NS_UNCONSTRAINEDSIZE before invoking comparison operator.
Differential Revision: https://phabricator.services.mozilla.com/D129230
ReflowOutput can accept any writing mode or ReflowInput passed to its
constructor, so a caller can pass a writing mode that is more convenient
to retrieve the size.
After reflowing flex items, we want to cache its metrics in flex item's
writing mode, so we pass item's ReflowInput into ReflowOutput. However,
for out-of-flow children, we can change ReflowOutput to use flex
container's writing mode by convention.
Differential Revision: https://phabricator.services.mozilla.com/D127913
Currently, we assume any flex item with percentage padding will be
marked as dirty if the percentage basis is changed. However, it is not
true.
To fix it, we cache the flex item's border and padding used in its most
recent final reflow to detect their changes.
dynamic-isize-change-004.html is designed to catch the concern in bug
1700580 comment 12.
Differential Revision: https://phabricator.services.mozilla.com/D125620
This patch introduces a class that caches metrics in flex item's final
reflow. With it, we have a better symmetry for caches, one for measuring
reflow and the other for final reflow. Also, we don't have to explain
that mLastReflowTreatedBSizeAsIndefinite is only meaningful if
mFinalReflowSize is set.
Differential Revision: https://phabricator.services.mozilla.com/D124967
Add `CachedFlexItemData::Update()` with the same arguments as the
constructor to update the existing cache for both the measuring & final
reflow.
`mFinalReflowSize` is going to be renamed in a later patch, so I also
change `UpdateFinalReflowSize()` to `Update()` for consistency with the
new API.
Differential Revision: https://phabricator.services.mozilla.com/D124966
Similar to bug 1728319, a huge main gap size can also make SumOfGaps()
negative due to integer overflow. This patch is to recognize that
scenario.
We still need to annotate the crashtest because it still triggers the
following assertion in nsIFrame.
```
ASSERTION: inline-size less than zero: 'result >= 0'
```
Differential Revision: https://phabricator.services.mozilla.com/D125778
We could fix the testcase in this patch by promoting
`FlexLine::mTotalItemMBP` to `AuCoord64`, but then
`layout/generic/crashtests/1488762-1.html` will start crashing. It is
because `FlexItem::OuterMainSize()` can return negative size due to
integer overflow, and the negative sizes will accumulate in
`mTotalOuterHypotheticalMainSize`.
Instead of promoting more variables to `AuCoord64` to workaround huge
margin/border/padding that we cannot handle gracefully, this patch
tweaks the assertion to check only if `mTotalOuterHypotheticalMainSize`
and `mTotalItemMBP` have valid values.
Differential Revision: https://phabricator.services.mozilla.com/D124409
The assertion is testing the sign of `availableFreeSpace` and `isUsingFlexGrow`.
After we use 64-bit arithmetic, it's likely that the stronger assertion holds.
Differential Revision: https://phabricator.services.mozilla.com/D123519
The idea of this patch is to use 64-bit coord type when resolving the
main size for flex items to avoid integer overflow when individual flex
items have huge hypothetical main sizes, which can happen with
percent-width table-layout:fixed descendants. We have to avoid integer
overflow to shrink flex items properly in that scenario.
Delete the "Note:" in `AddLastItemToMainSizeTotals()` since we remove
`AddChecked()` in favor of regular 64-bit addition for
`mTotalOuterHypotheticalMainSize`.
The wpt testcase is adapted from bug 1469649 comment 12.
Differential Revision: https://phabricator.services.mozilla.com/D123268
The precision of `double` is needed when we are distributing large
`sizeDelta` via `availableFreeSpace * myShareOfRemainingSpace`.
Without this patch, the wpt test added in Part 2 will fail.
Differential Revision: https://phabricator.services.mozilla.com/D123267
In an incremental reflow, if a flex item has a valid bsize cache , we
skip its measuring reflow. However, we may still need to set relevant
bsize flags if the flex container is changing its definiteness in the
block-axis. See bug 1611303 comment 2 for an analysis.
This patch is playing safe by always calling SetHasBSizeChange() if we
override bsize for the item. Of course this can be solved in a more
sophisticated way by checking whether the item really has a block-axis
resize, but that means we'll need to duplicate a lot of logic in
FlexItem::NeedsFinalReflow().
Differential Revision: https://phabricator.services.mozilla.com/D122041
CachedBAxisMeasurement::mAscent caches the ascent of a flex item after
the measuring reflow, but the ascent may change after the final reflow
if the item is stretched and does some vertical alignment internally.
However, we don't cache the new ascent. Therefore, when we reflow the
item incrementally, if the CachedBAxisMeasurement::Key is valid, we just
skip the measuring reflow, and retrieve the wrong ascent from the cache.
Instead of fixing this bug by updating the cached ascent or rejecting
the ascent cache for a stretching flex item in block axis, this patch
removes the cache and sets ReflowOutput's BlockStartAscent() to the flex
item after the item's measuring reflow. (We've done the same after the
item's final reflow.) If the ascent is ReflowOutput::ASK_FOR_BASELINE,
we resolve in FlexItem::ResolvedAscent() anyway.
Differential Revision: https://phabricator.services.mozilla.com/D121404
Our earlier call to nsLayoutUtils::GetFirstLineBaseline/GetLastLineBaseline
works in most cases, but those APIs don't handle every frame type and fails for
text control frames (for example). This new call should handle those cases
by directly asking the frame for its baseline.
Differential Revision: https://phabricator.services.mozilla.com/D121922
This patch doesn't affect behavior; it just refactors some logic to have an
early-return and reduce indentation, to make the next patch in this series
easier/simpler.
While we're at it, this patch also updates & extends some neighboring
code-comments to be more specific & more correct about how this code behaves
and its limitations.
Differential Revision: https://phabricator.services.mozilla.com/D121921
When a flex item's subtree is dirty, we should reject the cached
measurement and perform a measuring reflow for the item to get the
correct ascent. Without this patch, we are going to call
FlexItem::ResolvedAscent() in the flex algorithm based on a dirty flex
item subtree, and get a wrong ascent. (Although we'll still perform the
final reflow for the item, it's too late to get the correct ascent to
compute flex container's ascent.)
With the above modification, it exposes an existing issue that
layout/generic/crashtests/1666592.html can now trigger
`MOZ_ASSERT(!mFinalReflowSize)` in UpdateFinalReflowSize() in
fragmentation scenario. The problem is: when we clone a FlexItem for a
child frame in a flex container's continuation via FlexItem::CloneFor(),
we didn't reset the mHadMeasuringReflow flag, so we wrongly assume it
had a measuring reflow and may update its final reflow size based on the
cached metrics. However, we never run measuring reflow for flex items in
flex container's continuation, so we should reset the flag to prevent it
from falling into wrong path in FlexItem::NeedsFinalReflow().
Differential Revision: https://phabricator.services.mozilla.com/D121405
