Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LayoutNG] Nested fixedpos in nested multicol w/ CB in outer multicol #28634

Merged
merged 1 commit into from Apr 22, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Apr 22, 2021

Nested fixedpos elements in a nested multicol were handled previously
by CL:2791850 and CL:2823871. However, if a fixedpos was nested in an
OOF element inside a nested multicol, and the containing block for
the fixedpos was in an outer multicol, the fixedpos was never laid out.
The reason being that when we add an inner multicol with pending OOFs,
we don't make any changes to its OOF descendants. So if the fixedpos
containing block existed outside the inner multicol but inside the
outer multicol, fixedpos_containing_block in NGOutOfFlowPositionedNode
would never be set.

In order to fix this, we need to store a fixedpos_containing_block along
with any inner multicols with pending OOFs, if one exists. A new
struct, NGMulticolWithPendingOOFs, was added to store this information.
The fixedpos_containing_block gets set the first time the inner multicol
reaches a potential fixedpos containing block.

The multicol_offset variable of NGMulticolWithPendingOOFs was added for
two purposes:

  1. Similarly to CL:2836226, the static position was incorrect for
    nested fixedpos elements in a nested fragmentation
    context, and whose containing block was outside both multicols.
    (See fixed-in-nested-multicol-with-transform-container.html for an
    example).
    In this case, multicol_offset is the accumulate multicol offset
    relative to the outer fragmentation context. This gets added
    to the static pos of any fixedpos descendants in
    NGContainerFragmentBuilder::TransferOutOfFlowCandidates().
  2. In the case where the inner multicol found a fixedpos containing
    block, multicol_offset will be the offset from the multicol
    to the top of the fixedpos containing block. This is also used
    in NGContainerFragmentBuilder::TransferOutOfFlowCandidates() to
    ensure that the fixedpos static position is correctly updated.

MulticolCollection was updated from a HeapHashSet of inner multicol
LayoutBoxes to a HeapHashMap from inner multicol LayoutBoxes to
NGMulticolWithPendingOOFs.

In order to avoid creating two NGMulticolWithPendingOOFs structs (one
for the logical space and one for the physical space), the
NGContainingBlock struct was also updated to a templated struct.

Note: this CL does not handle the case of more than two layers
of nesting. For example, in the following case, the fixedpos would
not be laid out at the correct offset:
multicol
multicol
transform
multicol
relpos
abspos
fixedpos

In order to get this case working, we'd also need to store a reference
to the multicol that the fixedpos containing block was found in inside
NGMulticolWithPendingOOFs. However, this use-case doesn't seem worth
handling at this time.

Bug: 1198674,1200324
Change-Id: I213c35514fecf84fc62e55fe5531cff8b0830429
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2844299
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875257}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2844299 branch 2 times, most recently from d1b92ad to 05f71c5 Compare April 22, 2021 16:17
Nested fixedpos elements in a nested multicol were handled previously
by CL:2791850 and CL:2823871. However, if a fixedpos was nested in an
OOF element inside a nested multicol, and the containing block for
the fixedpos was in an outer multicol, the fixedpos was never laid out.
The reason being that when we add an inner multicol with pending OOFs,
we don't make any changes to its OOF descendants. So if the fixedpos
containing block existed outside the inner multicol but inside the
outer multicol, fixedpos_containing_block in NGOutOfFlowPositionedNode
would never be set.

In order to fix this, we need to store a fixedpos_containing_block along
with any inner multicols with pending OOFs, if one exists. A new
struct, NGMulticolWithPendingOOFs, was added to store this information.
The fixedpos_containing_block gets set the first time the inner multicol
reaches a potential fixedpos containing block.

The multicol_offset variable of NGMulticolWithPendingOOFs was added for
two purposes:
  1. Similarly to CL:2836226, the static position was incorrect for
    nested fixedpos elements in a nested fragmentation
    context, and whose containing block was outside both multicols.
    (See fixed-in-nested-multicol-with-transform-container.html for an
    example).
    In this case, multicol_offset is the accumulate multicol offset
    relative to the outer fragmentation context. This gets added
    to the static pos of any fixedpos descendants in
    NGContainerFragmentBuilder::TransferOutOfFlowCandidates().
  2. In the case where the inner multicol found a fixedpos containing
    block, multicol_offset will be the offset from the multicol
    to the top of the fixedpos containing block. This is also used
    in NGContainerFragmentBuilder::TransferOutOfFlowCandidates() to
    ensure that the fixedpos static position is correctly updated.

MulticolCollection was updated from a HeapHashSet of inner multicol
LayoutBoxes to a HeapHashMap from inner multicol LayoutBoxes to
NGMulticolWithPendingOOFs.

In order to avoid creating two NGMulticolWithPendingOOFs structs (one
for the logical space and one for the physical space), the
NGContainingBlock struct was also updated to a templated struct.

Note: this CL does not handle the case of more than two layers
of nesting. For example, in the following case, the fixedpos would
not be laid out at the correct offset:
  multicol
    multicol
      transform
        multicol
          relpos
            abspos
              fixedpos

In order to get this case working, we'd also need to store a reference
to the multicol that the fixedpos containing block was found in inside
NGMulticolWithPendingOOFs. However, this use-case doesn't seem worth
handling at this time.

Bug: 1198674,1200324
Change-Id: I213c35514fecf84fc62e55fe5531cff8b0830429
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2844299
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#875257}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants