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

Need to propagate tallest unbreakable before forced breaks. #31313

Merged
merged 1 commit into from Oct 19, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 19, 2021

Even if there's a forced break inside the child added to the fragment
builder, we need to propagate the tallest unbreakable block-size, to get
column balancing right.

This fixes an existing test, but I felt that we needed a test that more
clearly illustrates this fix, so added that. The existing test still
fails in legacy layout, but for a different reason (layout is correct,
but outline painting order is wrong).

Took the opportunity to rename PropagateBreak() to PropagateBreakInfo(),
since this method doesn't only care about actual breaks. Also, no need
to call this one if no block fragmentation is involved.

Bug: 829028
Change-Id: Ie951817095267567b9fae578fd519b4ba76dc24c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3231178
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#933116}

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.

Even if there's a forced break inside the child added to the fragment
builder, we need to propagate the tallest unbreakable block-size, to get
column balancing right.

This fixes an existing test, but I felt that we needed a test that more
clearly illustrates this fix, so added that. The existing test still
fails in legacy layout, but for a different reason (layout is correct,
but outline painting order is wrong).

Took the opportunity to rename PropagateBreak() to PropagateBreakInfo(),
since this method doesn't only care about actual breaks. Also, no need
to call this one if no block fragmentation is involved.

Bug: 829028
Change-Id: Ie951817095267567b9fae578fd519b4ba76dc24c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3231178
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#933116}
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