Closed Bug 1465771 Opened Last year Closed 6 months ago

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

Categories

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

Unspecified
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=283f1243ed8f18c0e10d44baaa7e67fe440183d2.
Attachment #8983123 - Flags: review?(jwatt)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
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.
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 jkew@mozilla.com:
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
Flags: needinfo?(jwatt)
Attachment #8983123 - Attachment is obsolete: true
Attachment #8983124 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16504 for changes under testing/web-platform/tests
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.