Bug 35583 – if font-family lists several generic families, the last one takes precedence

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

[Olivier Tilloy]

According to the CSS fonts specification (http://www.w3.org/TR/css3-fonts/#font-family-the-font-family-property), "[the font-family property] specifies a prioritized list of font family names or generic family names", and "authors are encouraged to append a generic font family as a last alternative for improved robustness".

It doesn't make much sense to list several generic families in a font-family property, since all generic font families are guaranteed to be always available by the user agent. However if one does, the current behaviour of webkit is to override the fallback family with the last generic family encountered in the list.
Consider the following example:

    font-family: ThisFontIsInvalid, monospace, sans-serif

If the first font family is not found on the system, the font selected should be the UA's default monospace font. Instead, it is the default sans-serif font.

A real life example is at http://svn.debian.org/wsvn/secure-testing/doc/narrative_introduction.
The text of the page is intended to be displayed with a fixed width, and the the style is defined like this:

    font-family:'Consolas',monospace,sans-serif;

(in http://svn.debian.org/websvn/templates/calm/styles.css)

[Olivier Tilloy]

Created attachment 49808 [details]
layout test case

This is a layout test case that highlights the issue.
I used the offsetWidth property of inline <p> elements to determine whether the font used for the two paragraphs is the same, if there is a more elegant solution it is welcome.
I guess this test could go in LayoutTests/fast/css.

[Olivier Tilloy]

Created attachment 49810 [details]
[patch] set the generic family only once

This patch fixes the issue by setting the generic family only once.
If another generic family is encountered in the list, setGenericFamily() is not called again.

[Alexey Proskuryakov]

The test says "PASS" for me. I was testing in Safari on Mac OS X.

OIf the behavior is indeed correct in some ports, it's likely that a proper fix wouldn't need to touch cross-platform code.

Please see <https://webkit.org/coding/contributing.html> for detailed information about contributing code to WebKit - we do require creating a single patch with tests and ChangeLogs.

[Olivier Tilloy]

Thanks for your feedback Alexey.
It looks like the bug I'm trying to fix is indeed platform-specific. Since I could reproduce it on the GTK and QT ports on linux, I assumed a bit too fast it was general.

Still, I believe the first part of my analysis and the patch are valid, as far as I can interpret the specification and understand the CSSStyleSelector code correctly. I can't think of an obvious layout test for it though.

If anyone who knows better this part of the code could take a look into it, it would be much appreciated! If my change makes sense I'll gladly generate a suitable patch with the Changelog.


As far as the layout test I attached is concerned, my guess is that ports that do the right thing (like the mac one) iterate over the list of families, discarding the first one as invalid and falling back on the second one ("-webkit-monospace"), whereas the GTK port seems to always generate a font from the first family, using the family name and falling back to the font description's generic family (which explains why this patch seems to fix it).
I will file a separate bug for the GTK port.

[Alexey Proskuryakov]

> If anyone who knows better this part of the code could take a look into it, it
> would be much appreciated!

A bug without any patches marked for review has pretty low visibility. Please consider either submitting a patch for review (as documented at https://webkit.org/coding/contributing.html), or e-mailing webkit-dev.

[Olivier Tilloy]

Created attachment 49919 [details]
Updated patch with Changelog entry

[Darin Adler]

Comment on attachment 49919 [details]
Updated patch with Changelog entry

Even if you can't test with DumpRenderTree cross-platform you can do the following:

    1) Add a DumpRenderTree test that requires per-platform results.

    2) Create a manual test with instructions for how to interpret the results and put it in WebCore/manual-tests.

    3) Add some new feature to DumpRenderTree to make it possible to test this across platforms.

As another possibility, is there perhaps some way to detect which family is used with computed style?

It's not OK to make the change without a test at all, so you need to do at least one of the things above.

[Olivier Tilloy]

(In reply to comment #7)
> (From update of attachment 49919 [details])
> Even if you can't test with DumpRenderTree cross-platform you can do the
> following:
> 
>     1) Add a DumpRenderTree test that requires per-platform results.
> 
>     2) Create a manual test with instructions for how to interpret the results
> and put it in WebCore/manual-tests.
> 
>     3) Add some new feature to DumpRenderTree to make it possible to test this
> across platforms.

#3 looks like the best option, but also the most complex to implement (given my
current knowledge of the code). Also, wouldn't changing the output of
DumpRenderTree break a lot of existing tests?

> As another possibility, is there perhaps some way to detect which family is
> used with computed style?

No, I already tried that, but the computed style only contains an ordered list
of font families as encountered by the CSS parser, no hint on which of them is
actually used.
The fact that webkit chooses to remember only one generic family instead of a
list of them is an implementation detail that's unknown to the CSS specification.

So I noticed that

platform/graphics/chromium/FontCacheChromiumWin.cpp
platform/graphics/chromium/FontCacheLinux.cpp
platform/graphics/gtk/FontPlatformDataGtk.cpp
platform/graphics/gtk/FontPlatformDataPango.cpp
platform/graphics/wx/FontPlatformDataWx.cpp

are the only files in platform/graphics that refer to the generic family in the font description. The Mac, Windows and Qt implementations don’t do this. I think this is the problem. The mapping from generic family to concrete family happes through the -webkit-* family names (e.g. -webkit-serif) and the Settings object in CSSFontSelector. I don’t know why the aforementioned ports don’t take advantage of that and do platform-level mapping.

[Ahmad Saleem]

All browsers (Safari 15.6 on macOS 12.5, Chrome Canary 106 and Firefox Nightly 104) show "PASS" in the attached test case, if it this desired output then I think this bug can be marked "RESOLVED CONFIGURATION CHANGED".

NOTE - Safari do fail some 'font-family' css test cases as shown in WPT:

https://wpt.fyi/results/css/css-fonts?label=master&label=experimental&aligned&q=font-family