Closed Bug 1749974 Opened 4 years ago Closed 4 years ago

[wpt-sync] Sync PR 32366 - Fix fragment post-layout cloning for multicol.

Categories

(Core :: Layout: Columns, task, P4)

Product:
Core
Component:
Layout: Columns
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
98 Branch
Tracking Flags:
Tracking Status
firefox98 --- fixed

People

(Reporter: wpt-sync, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 32366 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/32366
Details from upstream follow.

Morten Stenshorne <mstensho@chromium.org> wrote:

Fix fragment post-layout cloning for multicol.

Fragment post-layout cloning is needed for two things:

  1. Rebuilding the fragment tree spine after subtree relayout
  2. Recalculating overflow without laying out

Don't store layout results on the flow thread. This was tricky to
maintain and hard to reason about (we'd store fragmentainer children of
the multicol container fragment, but not spanners, OOFs, list item
markers etc.), Besides, we're going to remove the flow thread concept as
soon as we can (which admittedly isn't that soon, though), so this
will prepare us for the future. This also means that we should no longer
set up FragmentData entries for the flow thread, so removed that code as
well.

When cloning a fragmentation context root fragment, we'll now also
update the children of its fragmentainers to the post-layout fragments,
since fragmentainers no longer have the concept of "post-layout" (line
boxes don't have that, either, FWIW).

For subtree relayout, all we need to do is making sure that we only
visit NG containing blocks when rebuilding the fragment tree spine (i.e.
skip any flow threads).

For overflow recalculation, we're not making any specific changes in
this CL at all; things will just start working (with at least one
exception, but I'll get back to that in a follow-up), and stop crashing.

The change in OwnerLayoutBox() (to never do anything special) would
trigger a DCHECK failure in OffsetFromOwnerLayoutBox() when hit-testing
(because the owner is the multicol container and |this| is a
fragmentainer), which is why we will now never hit fragmentainers
during hit-testing (and there should be no need for that anyway).
Add an additional DCHECK (and documentation) to
OffsetFromOwnerLayoutBox(), to make it clear that fragmentainers aren't
supported.

NGOutOfFlowLayoutPart::ReplaceFragmentainer() became a bit fancy, but it
was the best I could come up with. This part was easier when we could
just update fragments in the flow thread.

Had to update the unit tests that expected any useful FragmentData from
flow threads.

Bug: 1279078, 1279525
Change-Id: I1d1c8366fe22f75e9a56a8e3f9910671f1ee81bc

Reviewed-on: https://chromium-review.googlesource.com/3378669
WPT-Export-Revision: 6a47d9333e9409e555711a9e5bf38ce98b91c88f

Component: web-platform-tests → Layout: Columns
Product: Testing → Core
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.
Test result changes from PR not available.

https://hg.mozilla.org/mozilla-central/rev/b676ee0a158f
https://hg.mozilla.org/mozilla-central/rev/09c82ee6e6b3

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox98: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch
You need to log in before you can comment on or make changes to this bug.