This patch shouldn't change the behavior.
* Rename BlockReflowState's `mFloatBreakType` to `mTrailingClearFromPIF` since
it's the cached result of `FindTrailingClear()` from the block's prev-in-flow.
Note that `mTrailingClearFromPIF` can have `StyleClear::Line`, which is
not a float clear value. We'll remove `StyleClear::Line` in a later part to
make this 100% correct.
* Rename `nsLayoutUtils::CombineBreakType` to `CombineClearType` as well as its
arguments.
* Simplify FindTrailingClear()'s implementation.
Differential Revision: https://phabricator.services.mozilla.com/D158221
Nowadays, nsFloatCache stores no extra information but `nsIFrame*`, which makes
nsFloatCacheList and nsFloatCacheFreeList are just singly and doubly linked
list of frames, respectively.
Most of this patch is a plain rewrite to use nsTArray APIs. There are two
rewrite that need more explanation.
1. `BlockReflowState::mFloatCacheFreeList` is served as a temporarily list to
avoid extra allocations of nsFloatCache. It takes the ownership of floats
from a line via FreeFloats(), and later reuses the cache in AddFloat().
We don't need this anymore, so we remove it.
2. In `PlaceBelowCurrentLineFloats()`, the old code removes a float which
needs to be pushed to next continuation from `mBelowCurrentLineFloats`.
After looking through all of the floats, the remaining of
`mBelowCurrentLineFloats` are moved to the line because they belong to the
line. In this patch, rather than removing the pushed floats one by one from
`mBelowCurrentLineFloats`, I use a new list to collect the floats that need
to be added to the line, and just clear `mBelowCurrentLineFloats` at the
end of the method.
Differential Revision: https://phabricator.services.mozilla.com/D157976
This also properly handles placeholder frames, and ensures that when checking
next/prev sibling we ignore placeholder frames.
To properly test this for multiple levels of page value propagation, we also
need to use FirstInFlow to get page values when checking for breaks in block
frames.
Differential Revision: https://phabricator.services.mozilla.com/D157175
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
This also adds a small post-processing step for frame-construction to ensure
that replaced frames (or more generally frames with content but no children)
have the auto page-name set on them.
When page-name value tracking is switched to be lazy, this post-processing step
will likely be redundant and can be removed.
Differential Revision: https://phabricator.services.mozilla.com/D152701
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
This also adds a small post-processing step for frame-construction to ensure
that replaced frames (or more generally frames with content but no children)
have the auto page-name set on them.
When page-name value tracking is switched to be lazy, this post-processing step
will likely be redundant and can be removed.
Differential Revision: https://phabricator.services.mozilla.com/D152701
It's always true, so remove it.
Add another pref to allow -webkit-line-clamp to work on all blocks
rather than just legacy -webkit-boxes, which seems something we should
try to look into, eventually.
Depends on D155181
Differential Revision: https://phabricator.services.mozilla.com/D155182
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
Some sites use text content that consists entirely of whitespace, which can
lead to unexpected rendering in High Contrast Mode - namely, backplating of
invisible characters. This commit adds logic that detects text nodes consisting
entirely of whitespace and reports their text area as 0. This commit also adds
a test for the behavior, and modifies an existing test to reflect the new
behavior.
Differential Revision: https://phabricator.services.mozilla.com/D155248
We can move a float frame into its block parent's PushedFloatsList during column
balancing when it cannot fit in the available block-size.
Later, when the column balancing algorithm reflows the last column in an
unconstrained block-size (in the very last reflow if needed [1]), we have to
reflow the line which contains the float's placeholder. The old code uses
`GetPrevInFlow() || GetNextInFlow()` to detect this scenario, which is
insufficient because the float's block parent might not have any continuation.
This patch uses `NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR` bit to detect this instead.
[1] https://searchfox.org/mozilla-central/rev/5c04fc7016eb7f52cf835d482f1125c8f139c959/layout/generic/nsColumnSetFrame.cpp#1144-1154
Differential Revision: https://phabricator.services.mozilla.com/D154860
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
The old code tweaks mIsTopOfPage in ReflowFloat(). However
`aAvailableSize.ISize()` (in containing block's writing-mode) always equals to
ContentISize() because it is computed via ComputeAvailableSizeForFloat().
Therefore, the only reason the floatRI's mIsTopOfPage can be `false` is when
some floats are already being placed, and we only check this in the
`!earlyFloatReflow` branch.
By tweaking mIsTopOfPage bit in FlowAndPlaceFloat(), we can also remove two
unused parameters in ReflowFloat().
This patch shouldn't change the behavior.
Differential Revision: https://phabricator.services.mozilla.com/D151457
The if-branch is incorrect because ShouldAvoidBreakInside() [1] is called via
the parent block instance with float child's ReflowInput as the argument. In
principle, the child frame should report break-before status if it cannot fit
the page/column and wants to avoid breaking. Removing this if-branch is
potentially a behavior change, but it is the correct thing to do. (Note that
ReflowFloat()'s caller FlowAndPlaceFloat() already handles
IsInlineBreakBefore(). If the float frame type already implements break-avoid,
we are fine.)
The else-if branch can never be reached except when the float is a letter frame,
and we've tested that scenario a few lines just before the this added assertion.
The code coverage also shows this branch in never reached.
https://coverage.moz.tools/#revision=e6e2286d2ac25001127a1cf54a87a95fb435c734&path=layout%2Fgeneric%2FnsBlockFrame.cpp&view=file&line=6682
[1] Note: ShouldAvoidBreakInside() is a protected member function, which can
only be called for concrete frame classes inheriting from nsContainerFrame.
Differential Revision: https://phabricator.services.mozilla.com/D151456
There is some advantage to creating the float's ReflowInput in
FlowAndPlaceFloat().
1. ReflowFloat() doesn't need to output the float's margin and offset via the
output arguments. FlowAndPlaceFloat() can get them from the local ReflowInput
directly.
2. Since we are going to reflow the float, we have to create a ReflowInput
anyway. FloatMarginISize() can take the ReflowInput and be simplified. No need
to waste time to create a separate SizeComputationInput.
Also, delete the comment "Pass floatRS so the frame hierarchy can be
used (redoFloatRS has the same hierarchy)" because I believe it is obsolete.
This patch also lays the foundation for other improvements in the later patches.
Differential Revision: https://phabricator.services.mozilla.com/D151455
The old code in AddFloat() used to call nsBlockFrame::ComputeFloatISize() to
compute a float's inline-size, compare it with current line's available
inline-size, and determine whether FlowAndPlaceFloat() should be called.
However, it doesn't handle an orthogonal float with an auto block-size.
Luckily, FlowAndPlaceFloat() already has logic dealing with orthogonal
floats (bug 1141867), so this patch defers the decision to place a float below
the current line until the float's margin inline-size is computed in
FlowAndPlaceFloat().
Differential Revision: https://phabricator.services.mozilla.com/D151209
This patch is a preparation for the next part, and doesn't change the behavior
yet.
FlowAndPlaceFloat() is used to return true and false. This patch changes its
return value `true` to `PlaceFloatResult::Placed` and `false` to
`PlaceFloatResult::ShouldPlaceInNextContinuation`.
In the next part, we'll move the logic dealing with "below the current line
floats" into FlowAndPlaceFloat(), and make it return
`PlaceFloatResult::ShouldPlaceBelowCurrentLine`.
Differential Revision: https://phabricator.services.mozilla.com/D151208
First of all, `nsBlockFrame::AdjustFloatAvailableSpace()` is misleading. It
doesn't adjust the argument `aFloatAvailableSpace` at all, nor does it use any
fields in nsBlockFrame. It simply returns the available space in the parent
block's content area. Thus, I move it into BlockReflowState, and have it return
the available size rather than a rect because a size is sufficient for reflowing
a float.
Also, nsBlockFrame::ReflowFloat() only cares about the available size, but not
the position of the available space, so it is sufficient to pass a LogicalSize
computed by the new method ComputeAvailableSizeForFloat().
In FlowAndPlaceFloat(), there is a loop searching for a wide enough band to
place the float. We don't need to adjust availSize every time mBCoord is changed
in the loop. We can just call ComputeAvailableSizeForFloat() to get a new
available size before reflowing the float in the `!earlyFloatReflow` branch.
This patch shouldn't change the behavior.
Differential Revision: https://phabricator.services.mozilla.com/D151207
In FlowAndPlaceFloat(), condense two identical assertions about float frame
parent into one.
FlowAndPlaceFloat() calls ReflowFloat() only once, either an early reflow (when
`earlyFloatReflow` is `true`) or a late reflow (when `earlyFloatReflow` is
`false`), so the reflow status is always fresh.
Differential Revision: https://phabricator.services.mozilla.com/D151203
aIsAdjacentWithBStart is used to clear the mIsTopOfPage bit for the ReflowInput
argument `aFrameRI`. However, ReflowBlock() already has aState argument to query
IsAdjacentWithBStart(), so there's no need for aIsAdjacentWithBStart argument.
Differential Revision: https://phabricator.services.mozilla.com/D150832
As the comment in the moved hunk said, a floating first letter frame's
incomplete status means that there is more content to be reflowed on the line.
If the block containing the floating first letter has `break-inside:avoid`
style, the old code would call `SetInlineLineBreakBeforeAndReset()`, and the
block's parent frame would misinterpret the status as "this block needs to be
push to the next fragment" even if we are *not* in a paginated environment.
Hence the assertion "Shouldn't be incomplete if availableBSize is
UNCONSTRAINED."
To fix this bug, we can reset the float's reflow status for a first letter frame
before checking `ShouldAvoidBreakInside`.
Differential Revision: https://phabricator.services.mozilla.com/D149712
This patch is based on Mats Palmgren's idea bug 1599159 comment 0.
When a child block frame reports inline-break-before status after being
reflowed, its size in the returned reflow output shouldn't matter because the
parent is expected to push it to the next page/column [1].
Here is the detail of the debug assertion if the workaround in fieldset frame is
removed. In nsBlockReflowContext::ReflowBlock(), it sets mReflowOutput::BSize()
to 0xdeadbeef, which is a negative number in 32-bits integer. Without the early
break of the loop in nsBlockFrame::ReflowBlockFrame(), then later in the loop we
are going to pass the negative block-size to
BlockReflowState::GetFloatAvailableSpaceForBSize(), which triggers a `aBSize>=0`
assertion further down in the callstack in nsFloatManager::GetFlowArea().
layout/reftests/pagination/fieldset-00G.html can trigger the above assertion
after removing the workaround in fieldset frame.
I don't expect this patch will change the behavior even if there are some float
elements interacting with this block. Without the early breaking out of the
loop, we still end up in [1].
[1] https://searchfox.org/mozilla-central/rev/ace2c59e6c56b2dcba25af1aa8903a5e7f9a5857/layout/generic/nsBlockFrame.cpp#4026-4035
Differential Revision: https://phabricator.services.mozilla.com/D148815
Note: The WPT test included in this test is intended to excercise cases that
are (newly) interoperable between WebKit, Blink, and Gecko (with this patch).
There are other related cases where browsers still disagree; I'll add
additional WPT tests for those cases in a later patch in this series.
Differential Revision: https://phabricator.services.mozilla.com/D145159
This patch does not impact behavior.
Before this patch, these variables' names are abbreviations of their original
type-name, "nsCSSOffsetState". We renamed that type to SizeComputationInput in
bug 1277129, and this patch just updates the variable-names to reflect that
renaming, so that now they're abbreviations of the new type-name, rather than
the old one.
This patch also takes this opportunity to move these variables declarations
closer to their usage, and in one case we convert a variable to be a temporary
anonymous value in the middle of another assignment.
Differential Revision: https://phabricator.services.mozilla.com/D145137
This patch does not impact behavior; it's just a handful of renamings and
documentation-updates.
Before this patch, our float-handling code has a bunch of variables and
functions with "replaced" in the name. But in fact these aren't necessarily
replaced; they're just block-level frames that elicit a "false" return-value
from "BlockCanIntersectFloats". This category *includes* replaced boxes, and
it also includes frames that establish a formatting context, e.g.:
scrollframes, flex/grid containers, flow-root, table.
This patch seeks to clarify things by renaming these to refer to the fact that
they "avoid" floats, e.g. s/replacedBlock/floatAvoidingBlock/.
I've included a comment to reference this term in the BlockCanIntersectFloats()
documentation (note that the "can intersect" concept is the opposite of
"avoids"). I've also renamed a few "aFrame" variables to
"aFloatAvoidingBlock", for a few functions that are explicitly dealing with
this scenario and only expect to be passed frames that are in this category.
Differential Revision: https://phabricator.services.mozilla.com/D145131
