546315 - Firefox choses wrong font for generic family with non-default font prefs

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[dauerbaustelle]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2) Gecko/20100207 Namoroka/3.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2) Gecko/20100207 Namoroka/3.6

I have recognized some weird behaviour with Firefox 3.6.

Normally, I would expect Firefox falls back to the next given font in CSS `font-family` description, but it doesn't.

See here http://ubuntu-pics.de/bild/42734/shot_Va698c.png for exmaple. (The displayed text is the CSS `font-family` value.) On the right you can see my font configuration.

I'm on Linux, so I don't have any 'Courier' font. For third paragraph I therefore expect Firefox to fall back on monospace. It doesn't. For last example, I expect Firefox to chose monospace (I have monospace installed, so...). It doesn't.

I tried out with WebKit - and WebKit choses the fonts differently. (But, not as I expected, either)

Reproducible: Always

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could you attach the HTML you used to take that screenshot?  (See the "Add an attachment" link on this page.)

Comment 2

[dauerbaustelle]

Attachment: font selection test case

Comment 3

[dauerbaustelle]

I have added three test cases, this shows up like this: http://ubuntu-pics.de/bild/42837/shot_FZb0OC.png

Comment 4

[Boris Zbarsky [:bzbarsky]]

What do the following commands return for you as output?

  fc-match monospace
  fc-match Courier
  fc-match "some completely invalid family"

Ccing some of the font folks.

Comment 5

[dauerbaustelle]

sh-4.1$  fc-match monospace
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Book"
sh-4.1$  fc-match Courier
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Book"
sh-4.1$  fc-match "some completely invalid family"
DejaVuSans.ttf: "DejaVu Sans" "Book"

Dunno why fontconfig choses DejaVu Sans Mono rather than Monospace for 'monospace', but that shouldn't matter as they are both monospaced fonts.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Confirming; at the very least we should be ending up with a monospace font for "Courier, monospace" given that output, and we don't seem to be.

Comment 7

[Karl Tomlinson (:karlt)]

The difference between "monospace" and "Courier, monospace" seems to be due to font.size.fixed.x-western (Edit -> Preferences -> Content -> [Fonts & Colors] Advanced -> [Monospace] Size).

Attachment 427093 [details] looks like good evidence for not having that pref (or at least defaulting to the same size on Linux).

Comment 8

[Karl Tomlinson (:karlt)]

Oh, I see that that is not the only difference.

Comment 9

[Karl Tomlinson (:karlt)]

What distro and version of fontconfig is here?
("Courier, monospace" looks like "monospace" on my system when courier is not installed and the font-size is specified in length units.)

What is the output of "fc-match Courier,monospace"?

Also, languages are involved in font selection.  Could there be any non-Latin languages in the locale?

Comment 10

[dauerbaustelle]

I'm currently running fontconfig 2.8.0-1 on Arch Linux.

$ fc-match "Courier, monospace"
VeraMono.ttf: "Bitstream Vera Sans Mono" "Roman"

(I'm getting confused, why on earth does fontconfig chose the Bitstream mono font rather than Monospace itself ._.)

I'm using en_US.utf8; I have configured locale-gen to generate de_DE.utf8 additionally although I'm not using it.

Comment 11

[dauerbaustelle]

Having the same problems again. On en.wikipedia.org, Firefox choses a non-monospace font (probably DejaVu Sans) for "monospace, sans-serif".

$ fc-match "monospace, sans-serif"
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Book"
$ fc-match "monospace"
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Book"
$ fc-match "sans-serif"
DejaVuSans.ttf: "DejaVu Sans" "Book"

Comment 12

[dauerbaustelle]

I did further research on this issue.

If I select 'DejaVu Sans Mono' as monospaced font in Firefox settings, I get that font for any CSS font-family definition that contains 'monospace'. If I select 'monospace' in Firefox settings, Firefox uses DejaVu Sans.

Comment 13

[Karl Tomlinson (:karlt)]

Thanks for the additional info.

Note that "monospace" is a fontconfig alias.  When this is selected in Firefox settings, fontconfig tries to select a suitable font for the language of the text.  e.g. monospace for Chinese may resolve to a different actual font than monospace for en_US.

The fontconfig "monospace" alias is actually turned into a large number of other family names, each family supporting some subset of languages/scripts.  This might include "DejaVu Sans Mono" and "AR PL KaitiM GB" for example.  These other family names are usually given a so-called "weak binding", which means that they will only be used if the font supports the requested language.  While "fc-match monospace:lang=en_US" might give "DejaVu Sans Mono", fc-match monospace:lang=zh" might give "AR PL KaitiM GB".

The problem reported in comment 11 is due to the weak binding changing the priority given to the font families.

Usually font families have a "strong" binding.  Because "DejaVu Sans" is selected in Firefox settings for "Sans-serif", this has a strong binding.  ("monospace" is also given a strong binding, but the families that it maps to get a weak binding.)

CSS "monospace, sans-serif" therefore becomes something like "DejaVu Sans Mono" (weak) , "DejaVu Sans" (strong), and some other families.  fontconfig gives the strong binding higher priority even though it is later in the list, and so the weak "DejaVu Sans Mono" doesn't get used.

Normally there is only one generic in a CSS font-family list and normally it is at the end of the list, so the lower priority weak binding for the generic does not change anything.  However, there are examples here where there is a family after the generic, making the incorrect priority apparent.

Comment 14

[Karl Tomlinson (:karlt)]

Comment 13 doesn't explain the problem reported in comment 0.
i.e. why "font-family: somecompletelyinvalidfamily, monospace" doesn't give a monospace font.
I haven't been able to reproduce that, though I have an older fontconfig.

Also, whitespace is significant for fc-match, so fc-match "somecompletelyinvalidfamily, monospace" should be different from fc-match "somecompletelyinvalidfamily,monospace".

Comment 15

[Karl Tomlinson (:karlt)]

With a bit of effort we could possibly avoid the problem in comment 13 by
resolving the fontconfig aliases (e.g. monospace) to the first family
supporting the language (or having a strong binding) and then giving that
family a strong binding.  It would be a bit quirky because for some languages
the aliases may be intended to resolve to multiple families.  e.g. a Japanese
user might like western fonts for Latin characters:

 <match target="pattern" >
  <test name="lang" compare="contains">
   <string>ja</string>
  </test>
  <test name="family" >
   <string>sans-serif</string>
  </test>
  <edit mode="prepend" binding="same" name="family" >
   <string>Bitstream Vera Sans</string>
  </edit>
  <edit mode="prepend" name="family" >
   <string>IPAPGothic</string>
  </edit>
 </match>

Reverting the Firefox "Sans-serif" setting to the default "sans-serif" would
workaround this issue on Wikipedia.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Er... why are we passing "monospace" through to fontconfig if it's set in CSS?  Shouldn't it be getting substituted with the user's default monospace font from preferences?  "monospace" is a generic-family, not a family-name, for the font-family CSS property.

Comment 17

[Karl Tomlinson (:karlt)]

(In reply to comment #16)
"monospace" is a generic for CSS and for fontconfig.  The default (CSS) monospace font from preferences is the fontconfig alias "monospace".
This allows us to pick up the user's preferred monospace font (which is usually the same as is used in other applications), without them having to set Firefox preferences (and similar preferences each other application).

Comment 18

[mikedoo3]

I'm seeing this problem on RHEL 5 and Cent OS 5.  
rpm -qa | grep -i fontconfig
fontconfig-devel-2.4.1-7.el5
fontconfig-2.4.1-7.el5

Firefox 2 did NOT have this problem on these systems.  I shouldn't have to hack the fontconfig configuration just to make Firefox work properly.

Comment 19

[Karl Tomlinson (:karlt)]

(In reply to comment #14)
> Comment 13 doesn't explain the problem reported in comment 0.
> i.e. why "font-family: somecompletelyinvalidfamily, monospace" doesn't give a
> monospace font.

As mentioned in bug 589779, I noticed that the style system is adding "serif" or "sans-serif" after the "monospace", so even when the document doesn't specify a family after the generic, there may be a subsequent generic.

I think I can fix this by either giving families after a fontconfig generic a lower priority or just trimming the family list at the first fontconfig generic.

Comment 20

[Karl Tomlinson (:karlt)]

Attachment: use weak bindings to give families after a generic lower priority

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Are we distinguishing a fontconfig generic from a CSS generic?  Because:
  font-family: "monospace"
and
  font-family: monospace
are different declarations in CSS; the latter is a CSS generic and the former is not.

Also, we should probably fix the style system to mess up the font lists less... assuming all the platform font backends are ok with it.

Comment 22

[Karl Tomlinson (:karlt)]

(In reply to comment #21)
> Are we distinguishing a fontconfig generic from a CSS generic?  Because:
>   font-family: "monospace"
> and
>   font-family: monospace
> are different declarations in CSS; the latter is a CSS generic and the former
> is not.

Thanks, the patch here is only dealing with fontconfig generics.
CSS generics have been resolved by preferences at this stage.
It is the fontconfig generics causing this problem.
(If all the prefs are set to real fonts then it works as expected.)

> Also, we should probably fix the style system to mess up the font lists less...
> assuming all the platform font backends are ok with it.

OK.  Bug 589779 (or a new bug) can cover that.

Comment 23

[Karl Tomlinson (:karlt)]

Attachment: reftest

Comment 24

[Karl Tomlinson (:karlt)]

Attachment: reftest v2

This test passes on tryserver machines.

Comment 25

[Jonathan Kew [:jfkthame]]

Comment on attachment 468632 [details] [diff] [review]
use weak bindings to give families after a generic lower priority

>-    for (PRUint32 i = 0; i < aFamilies.Length(); ++i) {
>+    PRUint32 i = 0;
>+    for (; i < aFamilies.Length(); ++i) {
>         NS_ConvertUTF16toUTF8 family(aFamilies[i]);
>         AddString(pattern, FC_FAMILY, family.get());
>+
>+        // fontconfig generic families are typically implemented with weak
>+        // aliases (so that the preferred font depends on language).  However,
>+        // this would give them lower priority than subsequent non-generic
>+        // families in the list.  To ensure that subsequent families do not
>+        // have a higher priority, they are given weak bindings.
>+        for (PRUint32 g = 0; g < NS_ARRAY_LENGTH(sFontconfigGenerics); ++g) {
>+            if (FcStrCmpIgnoreCase(ToFcChar8(sFontconfigGenerics[g]),
>+                                   ToFcChar8(family.get()))) {
>+                ++i;
>+                goto weak;
>+            }
>+        }
>+    }
>+
>+weak:
>+    for (; i < aFamilies.Length(); ++i) {
>+        NS_ConvertUTF16toUTF8 family(aFamilies[i]);
>+        AddWeakString(pattern, FC_FAMILY, family.get());
>     }

Could we do this without the "goto" and separate loop, please? E.g. by using a boolean flag useWeakBinding which starts out false, and is set to true if we find a generic, and call AddString or AddWeakString depending whether the flag is set.

Comment 26

[Karl Tomlinson (:karlt)]

Attachment: use weak bindings to give lower priority to families after a generic v2

Comment 27

[Jonathan Kew [:jfkthame]]

Comment on attachment 492435 [details] [diff] [review]
use weak bindings to give lower priority to families after a generic v2

Looks fine, thanks.

Comment 28

[Karl Tomlinson (:karlt)]

Comment on attachment 492435 [details] [diff] [review]
use weak bindings to give lower priority to families after a generic v2

approval2.0?
This fixes a bug which shows up as incorrect font selection when users set a subset of font family preferences (as well as in some less common situations).

Comment 29

[Benjamin Smedberg]

Comment on attachment 492435 [details] [diff] [review]
use weak bindings to give lower priority to families after a generic v2

Given how few beta testers we have on Linux to catch regressions, and that this is not a regression from Firefox 3.6, it doesn't appear that this is worth the risk.

Comment 30

[Karl Tomlinson (:karlt)]

Using a proportional font instead of a monospace font seems a significant bug to me, and the font pref UI encourages people to cause this to show up (because it presents only the serif font).

Landed just the test:
http://hg.mozilla.org/mozilla-central/rev/1907b6708e86

Comment 31

[Stefan]

1. Still Chooses wrong font
2. Presents monospace font in two different heights

checked with 
Built from http://hg.mozilla.org/mozilla-central/rev/114966e5b168

Comment 32

[Stefan]

Attachment: screenshot of testcase with rev 114966e5b168

Comment 33

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/032297452afd

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And also, Ahem...: https://hg.mozilla.org/projects/cedar/rev/29f08082b76f

Comment 35

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/032297452afd
http://hg.mozilla.org/mozilla-central/rev/29f08082b76f

Comment 36

[Hemiola SUN]

It seems that this bug 546315 is not really fixed. In many cases, the last line of the testcase is not correctly displayed. It is displayed with default font instead of Georgia font. Please see:

http://hemiolapei.free.fr/temp/fallback/fallback-test.html

I am not sure what causes exactly this bug. It occurs in Chinese and Japanese systems. In English system without any change of the language or font setting, it seems to work well.