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

Status: REOPENED

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

Description

[Jonathan Kew [:jfkthame]]

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.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: 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

Comment 2

[Jonathan Kew [:jfkthame]]

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

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: 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

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1611617 - Remove some invalid WPT tests for the 'ch' unit. r=heycam

Depends on D61189

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: 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

Depends on D61190

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1611617 - Add space glyph to munged versions of markA font used by broken-font testcases, and update embedded-data copies. r=heycam

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

Depends on D61191

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1611617 - In the font-face-unicode-range-2 testcase, add 'base' as fallback family after 'swoosh' for consistent line heights. r=heycam

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

Depends on D61192

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1611617 - Add missing U+0020 to unicode-range in CSS content-height reference files. r=heycam

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/21831 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1db18981975
Removed file font-face-unicode-range-2.html.ini a=wpt-fix

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/9f10c9424a1b
https://hg.mozilla.org/mozilla-central/rev/8f75f5c82eef
https://hg.mozilla.org/mozilla-central/rev/9cdffd787348
https://hg.mozilla.org/mozilla-central/rev/04651272912b
https://hg.mozilla.org/mozilla-central/rev/e09d13ba82eb
https://hg.mozilla.org/mozilla-central/rev/780285749c80
https://hg.mozilla.org/mozilla-central/rev/a1db18981975

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 15

[Jonathan Kew [:jfkthame]]

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.)