Open Bug 1611617 Opened 5 years ago Updated 2 years ago

A font without <space> is not valid to use as the "first available font" to compute normal line heights

Categories

(Core :: Layout: Text and Fonts, defect, P3)

Product:
Core
Component:
Layout: Text and Fonts
Type:
defect

Tracking

()

Status:
REOPENED
Milestone:
mozilla75
Tracking Flags:
Tracking Status
firefox75 --- affected

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 2 obsolete files)

Bug 1611617 - Fix incorrect test font ExTest.woff (does not support <space> character) used for css-values ex-unit testcase; add test that font without <space> is NOT used as basis for metrics. r=heycam
(deleted), text/x-phabricator-request
Details
Bug 1611617 - Remove some invalid WPT tests for the 'ch' unit. r=heycam
(deleted), text/x-phabricator-request
Details
Bug 1611617 - Regenerate mark* test fonts including a <space> glyph so that they are valid as the "first available font" for metrics purposes. r=heycam
(deleted), text/x-phabricator-request
Details
Bug 1611617 - In the font-face-unicode-range-2 testcase, add 'base' as fallback family after 'swoosh' for consistent line heights. r=heycam
(deleted), text/x-phabricator-request
Details
Bug 1611617 - Add missing U+0020 to unicode-range in CSS content-height reference files. r=heycam
(deleted), text/x-phabricator-request
Details

Various metrics depend on what the spec calls the first available font, which is specified to be the "the first available font that would match the U+0020 (space) character".

However, in some cases gfxFontGroup::GetFirstValidFont may return a font that does not actually support the character passed as its aCh argument, and this results in us getting metrics from a font that should not be used for this purpose.

We have several WPT failures in css/css-fonts/first-available-font-* as a result of this bug.

This fixes the first-available-font-* test failures; before tagging for review I want to check whether it disrupts anything else...

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED

Also add a -nospace version that does not include the space glyph, for use by the space-font-1 reftest.

Depends on D61191

Otherwise there's a risk that baseline positioning will vary depending which font is used to provide
the line-height metrics.

Depends on D61192

Attachment #9123056 - Attachment description: Bug 1611617 - Never let gfxFontGroup::GetFirstValidFont return a font from the font-family list that doesn't support the given character (prefer to fall back to the default font). → Bug 1611617 - Don't let gfxFontGroup::GetFirstValidFont return a font from the font-family list that doesn't support the given character (prefer to fall back to the default font).
Attachment #9123056 - Attachment description: Bug 1611617 - Don't let gfxFontGroup::GetFirstValidFont return a font from the font-family list that doesn't support the given character (prefer to fall back to the default font). → Bug 1611617 - Don't let gfxFontGroup::GetFirstValidFont return a font from the font-family list that doesn't support the given character (prefer to fall back to the default font). r=heycam
Attachment #9123341 - Attachment description: Bug 1611617 - Add struts to the test divs in font-face-unicode-range-2 to force consistent line height. r=heycam → Bug 1611617 - In the font-face-unicode-range-2 testcase, add 'base' as fallback family after 'swoosh' for consistent line heights. r=heycam
Attachment #9123340 - Attachment is obsolete: true
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9f10c9424a1b Fix incorrect test font ExTest.woff (does not support <space> character) used for css-values ex-unit testcase; add test that font without <space> is NOT used as basis for metrics. r=heycam https://hg.mozilla.org/integration/autoland/rev/8f75f5c82eef Remove some invalid WPT tests for the 'ch' unit. r=heycam https://hg.mozilla.org/integration/autoland/rev/9cdffd787348 Regenerate mark* test fonts including a <space> glyph so that they are valid as the "first available font" for metrics purposes. r=heycam https://hg.mozilla.org/integration/autoland/rev/04651272912b In the font-face-unicode-range-2 testcase, add 'base' as fallback family after 'swoosh' for consistent line heights. r=heycam https://hg.mozilla.org/integration/autoland/rev/e09d13ba82eb Add missing U+0020 to unicode-range in CSS content-height reference files. r=heycam https://hg.mozilla.org/integration/autoland/rev/780285749c80 Don't let gfxFontGroup::GetFirstValidFont return a font from the font-family list that doesn't support the given character (prefer to fall back to the default font). r=heycam
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/21831 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Pushed by dluca@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a1db18981975 Removed file font-face-unicode-range-2.html.ini a=wpt-fix
Upstream PR merged by moz-wptsync-bot
Regressions: 1616193
Regressions: 1616770
Regressions: 1616838

The code patch here was backed out in bug 1616268 due to perf regressions.

I have filed https://github.com/w3c/csswg-drafts/issues/4796 to reconsider the behavior currently required by the spec. (The test fixes remain valid, modulo any updates that may be needed if the spec gets changed.)

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9123056 - Attachment is obsolete: true
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: