feat: Support font-relative ch and ic units
#32171
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🔨 Triggering try run (#8876068432) for Linux WPT |
mrobinson
requested changes
Apr 29, 2024
|
Test results for linux-wpt-layout-2020 from try job (#8876068432): Flaky unexpected result (17)
Stable unexpected results that are known to be intermittent (17)
Stable unexpected results (57)
|
|
|
After servo#31966, which made it possible for the first time to resolve font-relative CSS units, this change adds support for the `ch` and `ic` units. One difference with the `ex` unit that was added in that PR is that these units must reflect the advance width of a character (the zero digit in the case of `ch`, and the CJK water radical for `ic`) as it would be rendered by the current font group. This means that the size of these units don't only depend on the first available font, in the case where that font does not contain a glyph for that character. This is implemented by adding the advance width for these two characters as optional fields of `FontMetrics`, so the advance width computation happens in advance. Then, when the font metrics are queried as part of unit resolution, the font group is searched for the first font containing that character. This change only implements support for these units in upright typesetting modes, since Servo does not yet have support for vertical writing modes. This means that many of the WPT tests that test for the behavior of these units with vertical writing modes do not pass. This change also makes a number of WPT tests pass, which relied on the `ch` and `ic` units. It, however, also makes the test `/css/css-text/white-space/text-wrap-balance-overflow-002.html` fail, since it tests `text-wrap: balance`, which Servo does not yet implement, and it was only previously passing by chance due to the previous behavior of these units.
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#46007) with upstreamable changes. |
|
🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#46007). |
mrobinson
approved these changes
May 1, 2024
Oh, I spoke to soon. Our scripts are clever enough to close this upstream PR without needing to squash. |
mrobinson
approved these changes
May 1, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Looks like there is one more consistently passing test: |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After #31966, which made it possible for the first time to resolve font-relative CSS units, this change adds support for the
chandicunits.One difference with the
exunit that was added in that PR is that these units must reflect the advance width of a character (the zero digit in the case ofch, and the CJK water radical foric) as it would be rendered by the current font group. This means that the size of these units don't only depend on the first available font, in the case where that font does not contain a glyph for that character.This is implemented by adding the advance width for these two characters as optional fields of
FontMetrics, so the advance width computation happens in advance. Then, when the font metrics are queried as part of unit resolution, the font group is searched for the first font containing that character.This change only implements support for these units in upright typesetting modes, since Servo does not yet have support for vertical writing modes. This means that many of the WPT tests that test for the behavior of these units with vertical writing modes do not pass.
This change also makes a number of WPT tests pass, which relied on the
chandicunits. It, however, also makes the test/css/css-text/white-space/text-wrap-balance-overflow-002.htmlfail, since it teststext-wrap: balance, which Servo does not yet implement, and it was only previously passing by chance due to the previous behavior of these units../mach build -ddoes not report any errors./mach test-tidydoes not report any errors