This separation makes hacking fragmentation code easier, and readers who are not
interested in flex container fragmentation can skip it.
Differential Revision: https://phabricator.services.mozilla.com/D163545
No need to be an out of band member function given the only callers are
in flex / grid code.
I was looking at this in the context of bug 1798396. This probably
doesn't matter for PGO, but it being a static method helps reason about
the flags being the same for all items.
Differential Revision: https://phabricator.services.mozilla.com/D161102
This fixes some tests with flex emulation enabled in the browser
toolbox (which I plan to enable soon enough in bug 1790616).
Differential Revision: https://phabricator.services.mozilla.com/D158629
Keep track of whether we have any collapsed items, since we're already
looking at it while generating them.
No behavior change intended.
Differential Revision: https://phabricator.services.mozilla.com/D158627
Make it work as a min-size everywhere rather than only handling it in
nsIFrame::ComputeSize.
This is more similar to what XUL code does. In particular, it makes
makes it work for elements that don't have a definite size in
ComputeSize (like flex containers).
For web-exposed widgets this only matters for <input type=color> and
<input type=range>, which are the only exposed widgets that return a
minimum size that return a minimum size. This patch makes the minimum
size honored for `<input type="color" style="height: auto">`. Add a test
for that (though we might want to change it to match other browsers).
Without this fix, min/max/close buttons on Linux are zero-height with
flex emulation, because we fail to account for their minimum
widget-imposed size.
This allows us to remove the special case in nsFlexContainerFrame too,
since that should be handled by ReflowInput correctly now.
Differential Revision: https://phabricator.services.mozilla.com/D158612
The XUL behavior in nsBox.cpp is fairly different to what the non-XUL
layout code paths do. In particular, canOverride=false means that the
min-{width,height} properties cannot go under the min widget size of the
widget, but that doesn't mean that intrinsic sizes don't affect the
final size of the widget.
This is very visible if you turn on flex emulation on Windows or macOS,
where the toolbar has an appearance that returns
width=0,height=N,canOverride=false.
With flex emulation we'd collapse the item to be zero-width, which is
not good at all.
The good thing is that this is no longer exposed to the web
(non-native-theme always returns canOverride=true), and our front-end
code doesn't seem to rely on this, so we can just remove support for
canOverride=false.
Differential Revision: https://phabricator.services.mozilla.com/D158608
Recently we added different bits for -webkit-/-moz- box emulation, so we
no longer need to go look at the scrolled frame etc.
Differential Revision: https://phabricator.services.mozilla.com/D158603
Instead of digging into the first line-iterable frame. Digging into the
first line-iterable frame is bogus, because if there are multiple flex
items we might prevent moving through them properly (see test-case).
The flex implementation is nice and fairly complete, IMO. The grid one
is not, but the resulting behavior is nicer than the behavior before
this patch, seems reasonable, and matches Chrome in my testing.
In Searchfox, the behavior is even funnier because user-select: none is
involved, but that predates the regression.
Differential Revision: https://phabricator.services.mozilla.com/D158086
Before this patch, we honored `align-content` (in combination with
`align-self`) for cross-axis alignment for abspos flex children **in cases
where the flex container was multi-line**. This was a bit weird, but was
required by the spec, and made some sense in the spirit of aligning the abspos
box as if it were the sole flex item in a flex container.
Now the CSSWG has resolved in [1] to simplify things by just ignoring
`align-content` for abspos flex children. So, this patch updates us in
accordance with this change. Such items now only have to look at `align-self`
for cross-axis alignment to determine their static position in their flex
container.
Before this commit, we had a bunch of WPT tests to check the impact of every
align-content value, with all of the various flavors of directionality. Now
that align-content has *no effect* in any of these cases, all of these tests
become pretty trivial and redundant. Rather than carrying them all forward
with trivial "no impact" expectations for each scenario, I've just updated the
first/simplest such test to expect no-behavioral-impact and I've removed the
rest.
[1] https://github.com/w3c/csswg-drafts/issues/7596#issuecomment-1225952646
Differential Revision: https://phabricator.services.mozilla.com/D157571
Some anonymous children are important for properly sizing their parents
even when those parents hide content with `content-visibility`. This is
shown by regressions in the proper layout of some form elements with
`content-visibility`.
This change introduces a more conservative approach for avoiding layout
of hidden content. Instead of leaving all children dirty during reflow,
reflow anonymous frames (and nsComboboxDisplayFrame, a specialized kind
of anonymous frame). This change means that frames may only lay out some
of their children, so it must introduce some more changes to assumptions
during line layout.
In addition, this change renames `content-visibility` related methods in
nsIFrame in order to make it more obvious what they do.
Differential Revision: https://phabricator.services.mozilla.com/D157306
Some anonymous children are important for properly sizing their parents
even when those parents hide content with `content-visibility`. This is
shown by regressions in the proper layout of some form elements with
`content-visibility`.
This change introduces a more conservative approach for avoiding layout
of hidden content. Instead of leaving all children dirty during reflow,
reflow anonymous frames (and nsComboboxDisplayFrame, a specialized kind
of anonymous frame). This change means that frames may only lay out some
of their children, so it must introduce some more changes to assumptions
during line layout.
In addition, this change renames `content-visibility` related methods in
nsIFrame in order to make it more obvious what they do.
Differential Revision: https://phabricator.services.mozilla.com/D157306
Some anonymous children are important for properly sizing their parents
even when those parents hide content with `content-visibility`. This is
shown by regressions in the proper layout of some form elements with
`content-visibility`.
This change introduces a more conservative approach for avoiding layout
of hidden content. Instead of leaving all children dirty during reflow,
reflow anonymous frames (and nsComboboxDisplayFrame, a specialized kind
of anonymous frame). This change means that frames may only lay out some
of their children, so it must introduce some more changes to assumptions
during line layout.
In addition, this change renames `content-visibility` related methods in
nsIFrame in order to make it more obvious what they do.
Differential Revision: https://phabricator.services.mozilla.com/D156473
ReflowChildren() only cares about the flex item's normal position, so we don't
need to output the relative position from `MoveFlexItemToFinalPosition()`.
By making `aFramePos` const for `MoveFlexItemToFinalPosition`, we can delete
`itemNormalBPos` because `framePos` cannot be changed after calling
`MoveFlexItemToFinalPosition`.
This patch shouldn't change the behavior.
Differential Revision: https://phabricator.services.mozilla.com/D157079
I haven't spotted any UI regression from this, and this should generally
make the XUL -> modern flex transition easier, and simplify some
of the relevant code.
This does fix a few layout issues with emulated flexbox.
For the most part, this shouldn't change behavior without that. This
changes behavior if you have mixed inline/non-inline content in the same
XUL box (before they'd get a single item, now you'd get the flexbox /
grid behavior of one item per inline run), and multiple inline-elements
(which would become their own flex items). But I pushed a patch with
some asserts and they didn't fire on our browser mochitests, so I think
we're good.
The UA rule refactoring (removing the inherit from xul anon blocks)
shouldn't matter in practice, since we only have one item (so
box-ordinal is irrelevant) and they have overflow: visible (so
text-overflow and overflow-clip-box shouldn't have an effect).
Differential Revision: https://phabricator.services.mozilla.com/D156375
This is a hack, sorta, similar to Chromium's:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/layout/layout_object.cc;l=356;drc=312b74e385e6aba98ab31fd911238c0dc16b396c
except at computed-value rather than used-value time, because it's both
simpler to reason about and prevents lying in the computed style.
This fixes the relevant test-case, and matches closer what Chromium does,
by not creating anonymous flex items for all elements inside the
line-clamp context.
The behavior change is covered by the test changes. I had to also fix a
couple pre-existing bugs that were caught by tests, now that the
line-clamped block is the -webkit-box-styled element rather than an anonymous
flex item (and thus now had padding).
Depends on D155180
Differential Revision: https://phabricator.services.mozilla.com/D155181
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
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
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
DONTBUILD
This patch doesn't impact behavior; it's just removing code-comments that refer
to a requirement in the spec that we don't honor, & which the CSSWG has now
resolved to remove (which makes our existing behavior correct & no longer
noteworthy).
The CSSWG resolution is in https://github.com/w3c/csswg-drafts/issues/4311 .
The new text is yet-to-be-written, but the main takeaway is that the "fully
inflexible" requirement (which we ignore) has been removed; so now, any flex
item with a definite flex-basis should have its main-size considered to be
definite. (And this matches our implementation.)
Differential Revision: https://phabricator.services.mozilla.com/D121284
The root cause of the crash was that FlexItem::NeedsFinalReflow
returned false even though the item had a non-empty next-in-flow.
This made the flex container skip the item's reflow and consider
the item's reflow status as COMPLETE, which triggers the removal
of the container's own next-in-flow, which causes the assertions
and eventually the crash.
Differential Revision: https://phabricator.services.mozilla.com/D118420
The rect `flexItemMarginBoxBounds` is initialized at position (0, 0),
not the flex container's content-box origin. Inflating its initial value
with flex container's padding is just wrong.
Differential Revision: https://phabricator.services.mozilla.com/D115854
Also, change the loop so that it iterates over flex items because
mFrames can contain out-of-flow placeholders which are not flex items.
This doesn't change the behavior because the out-of-flow placeholders
doesn't have any overflow areas (unless we fixed bug 1460484).
Differential Revision: https://phabricator.services.mozilla.com/D112672
This patch incorporates flex item's margin and flex container's padding
when computing flex container's overflow area in both the inline axis
and block axis.
overflow-top-left.html starts to fail because the test has flex items
with margin contributing to the overflow area now. We leave the test
unchanged for now until the webcompat situation is clear (Bug1698428).
flexbox-overflow-padding-002.html is based on
flexbox-overflow-padding-001.html with `writing-mode: vertical-rl` and
`direction: rtl` added to `.flexContainer`.
Differential Revision: https://phabricator.services.mozilla.com/D107936
This will prevent growing them when introducing fit-content(<length>).
This can _almost_ be derived, if it wasn't because of the quirky stuff.
I think the macro is probably good enough for now but let me know if you
disagree.
Differential Revision: https://phabricator.services.mozilla.com/D106713
This will prevent growing them when introducing fit-content(<length>).
This can _almost_ be derived, if it wasn't because of the quirky stuff.
I think the macro is probably good enough for now but let me know if you
disagree.
Differential Revision: https://phabricator.services.mozilla.com/D106713
