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

[line-clamp] Always clamp immediately after a line box #48482

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 4, 2024

In our initial implementation of line-clamp: auto, we allowed
clamping after lineless boxes, in order to better match the behavior
of line-clamp as it follows from the spec's continue: discard
model, even though our implementation uses a different model which is
not yet in the specification (see
w3c/csswg-drafts#7708).

However, in hallway conversations in TPAC 2024, we agreed that it
would be better for this model to instead only allow clamping
immediately after a line. If the first line does not fit, we calmp at
the first line even if that overflows the clamp height. If there are
no lines, we do not clamp. This patch implements this.

Additionally, a previous patch (https://crrev.com/c/5868971) had
increased the memory size of LineClampData from 8 to 20 bytes,
causing a memory regression. This patch shrinks it to 12 bytes, since
LineClampData no longer needs to keep track of how many block boxes
there are between the last line and the clmap point, and now OOFs
after the clamped line behave the same when clamping by a number of
lines or by a height, so there is no need for a flag to distinguish
these behaviors. This patch also adds a size assertion for
LineClampData.

This patch also changes or removes some WPT tests that were written
assuming the previous behavior.

Bug: 40336192, 368114054
Change-Id: I8c8afebb5dc566de92112cee2fcf24a2e60b42c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5904459
Reviewed-by: Ian Kilpatrick <[email protected]>
Commit-Queue: Andreu Botella <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1364495}

In our initial implementation of `line-clamp: auto`, we allowed
clamping after lineless boxes, in order to better match the behavior
of `line-clamp` as it follows from the spec's `continue: discard`
model, even though our implementation uses a different model which is
not yet in the specification (see
w3c/csswg-drafts#7708).

However, in hallway conversations in TPAC 2024, we agreed that it
would be better for this model to instead only allow clamping
immediately after a line. If the first line does not fit, we calmp at
the first line even if that overflows the clamp height. If there are
no lines, we do not clamp. This patch implements this.

Additionally, a previous patch (https://crrev.com/c/5868971) had
increased the memory size of `LineClampData` from 8 to 20 bytes,
causing a memory regression. This patch shrinks it to 12 bytes, since
`LineClampData` no longer needs to keep track of how many block boxes
there are between the last line and the clmap point, and now OOFs
after the clamped line behave the same when clamping by a number of
lines or by a height, so there is no need for a flag to distinguish
these behaviors. This patch also adds a size assertion for
`LineClampData`.

This patch also changes or removes some WPT tests that were written
assuming the previous behavior.

Bug: 40336192, 368114054
Change-Id: I8c8afebb5dc566de92112cee2fcf24a2e60b42c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5904459
Reviewed-by: Ian Kilpatrick <[email protected]>
Commit-Queue: Andreu Botella <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1364495}
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 merged commit 1563c1e into master Oct 4, 2024
21 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-5904459 branch October 4, 2024 23:52
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.

3 participants