1465771 - On Linux, a quoted font-family name may be incorrectly treated as a CSS generic-family

Status: RESOLVED FIXED

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

Description

[Jonathan Kew [:jfkthame]]

On Linux only (because of fontconfig-specific lookup code in gfxFcPlatformFontList::FindAndAddFamilies), we incorrectly treat quoted names such as "serif" or "fantasy" as if they were the CSS generics `serif`, `fantasy`, etc.

Per spec, the generics are (unquoted) keywords; a quoted font-family name is always a "real" family name to be looked up in the list of available fonts, not a generic to be mapped to an actual family via preferences or whatever mechanism.

Testcase: assuming there is no actual font family named "Fantasy" installed,

    data:text/html,<div style='font: 36px "fantasy", serif'>hello

should use the generic `serif` font (because "fantasy" is not found), while

    data:text/html,<div style='font: 36px fantasy, serif'>hello

should use whatever the `fantasy` generic maps to.

Currently, however, in both cases we use whatever fontconfig matching returns for the `fantasy` name (which on my rather plain Ubuntu machine just gives the system default sans-serif font, as nothing more interesting is configured).

(This will become worse with CSS Fonts 4 as it introduces additional generic-family keywords, increasing the risk that a quoted family name will accidentally clash and be misinterpreted as a generic.)

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Reftest for quoted font-family name that matches a CSS generic-family keyword

Here's a reftest that demonstrates the issue here, and fails on current trunk: https://treeherder.mozilla.org/#/jobs?repo=try&revision=283f1243ed8f18c0e10d44baaa7e67fe440183d2.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Avoid possibly treating a quoted font-family name as a CSS generic in the fontconfig-based backend

And a patch to resolve the problem, by passing the quoted state of the font-family name in to FindAndAddFamilies. Try run with the fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a57bbbd101d077f8dd9e3c2ffe5ff743016f3b7f.

Comment 3

[Jonathan Watt [:jwatt]]

Comment on attachment 8983123 [details] [diff] [review]
Reftest for quoted font-family name that matches a CSS generic-family keyword

Review of attachment 8983123 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/font-matching/quoted-generic-ignored.html
@@ +5,5 @@
> +/* Note that this test assumes that the system does not have an actual
> +   font named "Fantasy" installed! */
> +.test1 { font-family: "fantasy", serif; }
> +.test2 { font-family: "fantasy", sans-serif; }
> +.test3 { font-family: "fantasy", monospace; }

No 'cursive' test? ;)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Would there be any chance of putting the test in WPT? Seems like an mistake other engines could make too, and they run on linux unconditionally.

Comment 5

[Jonathan Kew [:jfkthame]]

I'm a bit hesitant to claim it's valid as a WPT test, given that it's dependent on the font environment where it runs; for reftests in our CI, I don't mind making a few assumptions about that, but strictly speaking the test is fragile in that way.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Hmm, ok, fair enough :). I still think it'd be useful, but not sure what's the policy about this kind of edge case.

Comment 7

[Jonathan Watt [:jwatt]]

FWIW I'd also err towards making the test a WPT. If other vendor's happen to run WPT in an environment with system fonts with names that match the generic family names (seems unlikely) they can always use their metadata system to disable that the test.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jfkthame, could you have a look please?

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1465771 - Avoid possibly treating a quoted font-family name as a CSS generic in the fontconfig-based backend. r=jwatt

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1465771 - Add a WPT-reftest for quoted font-family name that matches a CSS generic-family keyword. r=jwatt

Depends on D26916

Comment 11

[Jonathan Kew [:jfkthame]]

Just rebased the patch here, and moved the reftest over to WPT as suggested. :jwatt, would you mind r+'ing these again, so that Lando will be happy to consider them? Thanks.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb9dda98d12b
Avoid possibly treating a quoted font-family name as a CSS generic in the fontconfig-based backend. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/a7b675fcb8d3
Add a WPT-reftest for quoted font-family name that matches a CSS generic-family keyword. r=jwatt

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/cb9dda98d12b
https://hg.mozilla.org/mozilla-central/rev/a7b675fcb8d3

Comment 14

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

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

Comment 15

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

Upstream PR merged