Closed
Bug 1243226
Opened 9 years ago
Closed 9 years ago
gfxPlatformFontList for fontconfig neglects the fallback mechanism of fontconfig
Categories
(Core :: Graphics: Text, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: Hexcles, Assigned: jtd)
References
Details
Attachments
(3 files)
280 bytes,
text/plain
|
Details | |
479 bytes,
text/html
|
Details | |
1.46 KB,
patch
|
heycam
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20160126104524 Steps to reproduce: Note that the following steps, though seemingly peculiar, are only an example. Many other configurations may suffer from this bug (configuring font fallback in fontconfig is a common practice). I'll explain more in the comment later. On a Linux running Firefox 44.0 (stable): 1. Install the following fonts (all open-source and in the repos of major distributions): SourceSerifPro-Regular.otf (Source Serif Pro) uming.ttc (AR PL UMing CN) SourceHanSansCN-Normal.otf (思源黑体 CN) 2. Use the provided fonts.conf (copy to ~/.fonts.conf). 3. Check the output of fc-match (there might be some other fonts; just make sure the relative order of the mentioned fonts are correct.): fc-match -s "serif" > SourceSerifPro-Regular.otf: "Source Serif Pro" "Regular" > uming.ttc: "AR PL UMing CN" "Light" > ... fc-match -s "Source Serif Pro" > SourceSerifPro-Regular.otf: "Source Serif Pro" "Regular" > SourceHanSansCN-Normal.otf: "思源黑体 CN" "Normal" > ... (No UMing; or it appears AFTER Source Han Sans.) 4. Make sure gfx.font_rendering.fontconfig.fontlist.enabled is true (enabled by default in BUG 1180560). Actual results: For CSS font family "serif", Firefox renders western characters using "Source Serif Pro" (as expected), but mistakenly uses "Source Han Sans CN" for CJK characters (not following the fontconfig preferences). Expected results: For CSS font family "serif", Firefox should use "AR PL UMing CN" for CJK characters (as configured in ~/.fonts.conf).
Reporter | ||
Comment 1•9 years ago
|
||
I found a possible root cause: the current implementation of fontconfig platform fontlist lookup (added in BUG 1056479) breaks the fallback mechanism of fontconfig. With fontconfig, we are able to create a fallback list for generic families (say "serif") in case there are characters missing in some fonts. This is commonly used by CJK users as well as many distributions by default. However, the new gfxFcPlatformFontList::FindGenericFamily seems to simply find the first non-generic font in the list returned by fontconfig and map the generic font to that one (see https://bugzilla.mozilla.org/attachment.cgi?id=8603967&action=diff#a/gfx/thebes/gfxFcPlatformFontList.cpp_sec2 L1367), effectively neglect the fallback list. In the example above, I believe Firefox will map the generic "serif" to "Source Serif Pro". When a CSS demands "serif" font family, Firefox will now directly request "Source Serif Pro" to render. If the requested font misses some characters, fontconfig will still look for fallback fonts, but the fallback list for "Source Serif Pro" is not the same as for "serif".
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jd.bugzilla
Assignee | ||
Comment 2•9 years ago
|
||
(In reply to Robert Ma from comment #1) > However, the new gfxFcPlatformFontList::FindGenericFamily seems to simply > find the first non-generic font in the list returned by fontconfig and map > the generic font to that one (see > https://bugzilla.mozilla.org/attachment.cgi?id=8603967&action=diff#a/gfx/ > thebes/gfxFcPlatformFontList.cpp_sec2 L1367), effectively neglect the > fallback list. This logic was adjusted to support exactly the sort of thing you're trying to do here. See bug 1173260 for details. I'll work on reproducing this with your fonts.conf settings.
Assignee | ||
Comment 3•9 years ago
|
||
With this testcase I can reproduce the problem. Instead of the AR PL UMing being used for the substring of Chinese, a local sans-serif Japanese font is used.
Assignee | ||
Comment 4•9 years ago
|
||
The problem is that the while the generics code allows multiple fonts, when constructing the list it's limiting the list if a font is found for a given language: https://dxr.mozilla.org/mozilla-central/rev/aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/gfx/thebes/gfxFcPlatformFontList.cpp#1559 Robert's fonts.conf file is basically setting the mapping of all langs, so when a page with no lang setting is found and the app locale is en, only the first of the generics is included. This code is probably overly restrictive, best to just relax this and allow up to the limit (currently 3 fonts).
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8712534 -
Flags: review?(cam)
Assignee | ||
Updated•9 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 6•9 years ago
|
||
Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de350ab98251
Reporter | ||
Comment 7•9 years ago
|
||
(In reply to John Daggett (:jtd) from comment #4) > This code is probably overly restrictive, best to just relax this and allow up to the limit (currently 3 fonts). I'm wondering why we need to limit MaxGenericSubstitions. Is that a performance issue? In my real environment, I actually have more than five fallbacks for monospace (powerline symbol, emoji, western characters, and several CJK fonts). If we have to limit the length of the list for some reason, 3 seems to be a bit too small (as a side note, I've seen many websites using a font-family list with more than a dozen fallbacks). And thank you for looking into this!
Updated•9 years ago
|
Attachment #8712534 -
Flags: review?(cam) → review+
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Robert Ma from comment #7) > I'm wondering why we need to limit MaxGenericSubstitions. Is that a > performance issue? In my real environment, I actually have more than five > fallbacks for monospace (powerline symbol, emoji, western characters, and > several CJK fonts). If we have to limit the length of the list for some > reason, 3 seems to be a bit too small (as a side note, I've seen many > websites using a font-family list with more than a dozen fallbacks). The limit described here is simply the limit on the number of fonts we include when mapping a generic. There's no limit on the length of fontlists themselves. Fontconfig doesn't really provide easy ways to distinguish user preferences from the default font order. Including a lot of extra fonts slows down font fallback when it occurs, so putting in an artificial limit was best thing. If you want to increase the limit, you can bump it up using the pref below in about:config: gfx.font_rendering.fontconfig.max_generic_substitutions
Comment 10•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3cedb5df2b5d
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 11•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3cedb5df2b5d
Comment 12•9 years ago
|
||
Got another report of this issue. [1] :jtd, do you think we could uplift this patch to aurora and probably also beta so that it could be fixed soon for those users given it is a recent regression? [1] https://twitter.com/caomu/status/697060582327848961 (Chinese)
Flags: needinfo?(jd.bugzilla)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8712534 [details] [diff] [review] patch, relax the number of fontconfig generics Approval Request Comment [Feature/regressing bug #]: 1180560 [User impact if declined]: users with custom fontconfig settings for generics under Linux may not see the fonts they expect. Especially relevant to CJK Linux users. [Describe test coverage new/current, TreeHerder]: landed on trunk two weeks ago, no problems reported [Risks and why]: minor change, low impact, Linux only [String/UUID change made/needed]: none
Flags: needinfo?(jd.bugzilla)
Attachment #8712534 -
Flags: approval-mozilla-beta?
Attachment #8712534 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox45:
--- → affected
status-firefox46:
--- → affected
Comment 14•9 years ago
|
||
Comment on attachment 8712534 [details] [diff] [review] patch, relax the number of fontconfig generics Improve Linux support, taking it. should be in 45 beta 5
Attachment #8712534 -
Flags: approval-mozilla-beta?
Attachment #8712534 -
Flags: approval-mozilla-beta+
Attachment #8712534 -
Flags: approval-mozilla-aurora?
Attachment #8712534 -
Flags: approval-mozilla-aurora+
Comment 15•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/5bf2b052222d
Comment 16•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/11942efe7deb
Comment 17•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/11942efe7deb
You need to log in
before you can comment on or make changes to this bug.
Description
•