Closed Bug 1465771 Opened 3 years ago Closed 2 years ago

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


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




Tracking Status
firefox68 --- fixed


(Reporter: jfkthame, Assigned: jfkthame)


(Blocks 1 open bug)



(2 files, 2 obsolete files)

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.)
Here's a reftest that demonstrates the issue here, and fails on current trunk:
Attachment #8983123 - Flags: review?(jwatt)
Assignee: nobody → jfkthame
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:
Attachment #8983124 - Flags: review?(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? ;)
Attachment #8983123 - Flags: review?(jwatt) → review+
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.
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.
Hmm, ok, fair enough :). I still think it'd be useful, but not sure what's the policy about this kind of edge case.
Attachment #8983124 - Flags: review?(jwatt) → review+
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.

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?

Flags: needinfo?(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.

Flags: needinfo?(jfkthame) → needinfo?(jwatt)
Type: enhancement → defect
Pushed by
Avoid possibly treating a quoted font-family name as a CSS generic in the fontconfig-based backend. r=jwatt
Add a WPT-reftest for quoted font-family name that matches a CSS generic-family keyword. r=jwatt
Flags: needinfo?(jwatt)
Attachment #8983123 - Attachment is obsolete: true
Attachment #8983124 - Attachment is obsolete: true
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Created web-platform-tests PR for changes under testing/web-platform/tests
You need to log in before you can comment on or make changes to this bug.