379886 - Font preferences panel selects random fonts in cairo builds

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: Current trunk Firefox and Seamonkey builds on Linux.

STEPS TO REPRODUCE (Current trunk Firefox):

0)  Create a new profile, or use one where you didn't customize fonts
1)  Start the browser
2)  Edit > Preferences
3)  Select the "content" tab
4)  Click the "Advanced" button in the "Fonts & Colors" section
5)  Look at the "Fonts for" groupbox

EXPECTED RESULTS (as seen with 1.8 branch):
a)  Dropdown for "Serif" says "serif"
b)  Dropdown for "Sans-serif" says "sans-serif"

ACTUAL RESULTS:
a)  Dropdown for "Serif" says "AR PL KaitiM Big5"
b)  Dropdown for "Sans-serif" says "AR PL KaitiM Big5"

This causes two problems.  First of all, those fonts have nothing to do with what fonts are actually being used.  Secondly, if I happen to change some other setting in this pref panel and then hit "OK" I _will_ suddenly be using those fonts.  Which I don't want to be doing for "Western", trust me.  ;)

The thing is, if the preference is not set (default configuration), the fonts panel selects the first font in the list.  This _used_ to be the generic, but now the generic is alphabetized somewhere down near the bottom, so the first font is whatever starts with "A".  What that will be on your system might vary, of course.

In Seamonkey this regressed with the switch to cairo GFX, hence the component.

Comment 1

[Boris Zbarsky [:bzbarsky]]

At a guess, getDefaultFont() on the font enumerator is busted?  But it's not like the XFT one did anything either...  Maybe XFT itself put that font at the beginning?  Not sure.

Comment 2

[Boris Zbarsky [:bzbarsky]]

With bug 379888 fixed, the default-selected font is now Ahem.  That's not a great improvement, of course.  ;)

Comment 3

[Karl Tomlinson (:karlt)]

It seems that "Times" is used for "Serif" if available
(but, I'm not sure that's an improvement, as it is a bitmap font).
To reproduce I needed to disable Times in ~/.fonts.conf:

<?xml version="1.0"?>
<!DOCTYPE fontconfig SYSTEM "fonts.dtd">
<fontconfig>
 <selectfont>
  <rejectfont>
   <pattern>
     <patelt name="family"><string>Times</string></patelt>
   </pattern>
  </rejectfont>
 </selectfont>
</fontconfig>

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: put system configured generic fonts first in GetFontList()

Fixes the problems for those without Times installed.

(getDefaultFont() is not actually implemented.)

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: use system configured generic fonts as defaults for preferences

Fixes the problems for those with Times etc installed.

The system configured default fonts should be nicer than Times, Helvetica, and Courier pcf fonts.

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 275737 [details] [diff] [review]
use system configured generic fonts as defaults for preferences

Do we need ui approval or something on this?

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to comment #0)

> The thing is, if the preference is not set (default configuration), the fonts
> panel selects the first font in the list.

This seems the intentional behaviour of gFontsDialog.readFontSelection().
In contrast, however, gContentPane.init() seems to want to ensure that nothing is selected, presenting a blank menu item if the font is not available.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 275735 [details] [diff] [review]
put system configured generic fonts first in GetFontList()

This is ok, though this means that if someone has a bad font selected for, say, their sans serif font, we'll set it to 'monospace'.  That seems bad... not sure how to work around that, though.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> (getDefaultFont() is not actually implemented.)

Would it be possible to implement it?  That seems like a better idea in some ways...

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to comment #9)
> > (getDefaultFont() is not actually implemented.)
> 
> Would it be possible to implement it?

Well, I guess so, but:

* GetFontList() should still put the generic (system configured) first.

* It would be changing the current (and historic UI).

* I'm not sure it would be the UI that is wanted.

What you want is a reasonable font to use.  We could do this by making getDefaultFont() return serif, sans-serif, or monospace as appropriate.

With fontbuilder.js as is, the menu would then contain "Default(serif)" or similar as the first item, with value "".

My concern is that the "Default(...)" label makes it look like a mozilla default but in fact it is not.  [Times/Helvetica/Courier is the current mozilla default :(.]

[The backend (gfxFont) can't handle this value "" at the moment as it picks any font, not necessarily serif (or not necessarily monospace for monospace), but lets assume we could implement something for that.]

I s'pose, if we wanted to go this way, we could change "Default(...)" to "System Default(...)".

But implementing that would be a UI decision, which is not really this bug, and its probably not a blocker.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> * It would be changing the current (and historic UI).

Wasn't getDefaultFont() implemented on non-Linux platforms?

I agree that getDefaultFont() is a followup bug, for sure.

Comment 12

[Karl Tomlinson (:karlt)]

(In reply to comment #11)
> > * It would be changing the current (and historic UI).
> 
> Wasn't getDefaultFont() implemented on non-Linux platforms?

Not currently.  The only implementation is http://mxr.mozilla.org/mozilla/source/gfx/src/thebes/nsThebesFontEnumerator.cpp#115
(with no specializations of nsThebesFontEnumerator).
It's possible it once was implemented though.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I meant before Thebes landed, of course.  That is, if we're talking about UI change the relevant metric is from last (pre-Thebes) release.

We should spin all this into a separate bug, though.

Comment 14

[Karl Tomlinson (:karlt)]

And I still agree that showing an unselected menu item seems odd.
I don't know if that's a bug in the menulist implementation or in how it is
used.

But the content dialog behaviour that allows selecting "" from the blank entry
doesn't seem any better.
The preference at least is not set until a menu item is selected.

Comment 15

[Karl Tomlinson (:karlt)]

(In reply to comment #11)
> Wasn't getDefaultFont() implemented on non-Linux platforms?

In the firefox-2.0.0.6 implementations of nsFontEnumerator*::GetDefaultFont() under gfx/src, the closest to doing anything other than returning NULL was nsFontEnumeratorXft.  It had some "#if 0"'d code that got fontconfig to speak the actual family from the font it used for the generic family name.

Bug 209626 wants to activate this.
(I haven't decided whether its a good idea or not.)

Comment 16

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  If it never used to work, so much the simpler.  ;)

Comment 17

[Phil Ringnalda (:philor)]

gfx/thebes/src/gfxFontconfigUtils.cpp 1.5
modules/libpref/src/init/all.js 3.688

Comment 18

[[:Aleksej]]

Verified the expected results from comment #0 and monospace with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007082704 Minefield/3.0a8pre on Debian almost Lenny.

Comment 19

[Steve England [:stevee]]

I believe this caused bug 393890