206782 - [RFE] The Font setting pick up "[System Default]" for empty factory-default values

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Yuying Long]

Build: 05-22 trunk build on Mac OS 10.2.6

Steps to reproduce:
1. Launch browser, and go to Edit | Preferences | Appearance | Fonts.
2. Click on the Languages, e.g. Japanese, Western, Chinese ...etc.
3. Notice the Font names.

Result:
There are a lot of places are set to [System Default] instead of the real font
name.  And if you go some pages, e.g. home.netscape.com/ja, www.asahi.com, the
display are ugly during wrong fonts used.

This is a bad regression, I didn't see this on a few days (05-13) ago's build.

Comment 1

[rbs]

Some clarifications:
1) Mac specific?
2) You have to fo the Font panel to trigger the problem, right? That is, if you
don't go there, the problem doesn't happen. Am I understanding properly? [The
reason I am asking is to know if something is inavertandly overwritten.]
3) Does selecting nicer fonts (rather than leaving the default) bring back the
nicer rendering?

Comment 2

[Yuying Long]

1) Mac specific?
No.  I just installed a windows build, I had the same problem, but the only
differences are Mac and Windows are pick up different fonts for [System Default]. 

2) You have to fo the Font panel to trigger the problem, right? That is, if you
don't go there, the problem doesn't happen. Am I understanding properly? [The
reason I am asking is to know if something is inavertandly overwritten.]
No, browser pick up the "[System Default]" by default, you will see the problem
without go to Font Preferences.  E.g. by default, Japanese fonts are pick up
[System Default] except monospace; so if you go to a Japanese web page like
www.asahi.com, the display is ugly.

3) Does selecting nicer fonts (rather than leaving the default) bring back the
nicer rendering?
Yes.  But we shouldn't let default sets as [System Default] and show it in font
list, plus it's just a variable name.

Comment 3

[rbs]

I haven't yet debugged the matter, I will be looking deeper into it later, but
it is strange that the bug happens without even going to the Font pref pannel.
The shipped values of the prefs for |font.name.*| (which you can see with
about:config) are what the back-end uses. They can't be changed without going to
the pref pannel which is where changes normally occur to those prefs. I wonder
if your build wasn't using settings that were already contaminated by an earlier
trip to the Font pannel.

Comment 4

[Yuying Long]

I have tried new profiles, got the same result.

Comment 5

[rbs]

Very intriguing. On my Win2K system, the trunk and Nav7.02 render the same.
The fact that you observed the bug with:
- new profiles,
- not even changing the factory defaults,
- all platforms,
probably means that the problem isn't due to the Fonts pref pannel.

Comment 6

[rbs]

jshin, any clues here?

Comment 7

[Yuying Long]

> On my Win2K system, the trunk and Nav7.02 render the same.

What about font default setting in Preferences?
The render probably are same sometimes because a lot of pages has their own
style/fonts defined in their web page.  
But in latast trunk build, the font will pick up wrong default value.  Like I
said before, many Japanese pages in Mac are really not displayed nicely by
default which was not the case a few days ago.

Comment 8

[rbs]

> What about font default setting in Preferences?

The initial default settings come from the hard-coded ('factory') default values
that are shipped in winpref.js & friends.

With a brand new & fresh install, these hard-coded values are the ones normally
used by the back-end. You can see them with about:config at first launch
(without even going the the Fonts preference pannel). They can only be changed
if you go to the Fonts prefererence pannel, and again you can see the new values
in about:config.

However, since you are seeing the bug without even going to the Fonts
preferences, it is intriguing.

Comment 9

[Samir Gehani]

adt: nsbeta1+/adt1

Simon, please add an ETA to the whiteboard.  Thanks.

Yuying, could you narrow down between which two builds this first started
happening?  Thanks.

Comment 10

[Yuying Long]

I checked this on my Mac, [System Default] font appear happened between 05-20-08
and 05-21-03.

Btw, the display ugly problem was in one of my Mac machine which probably caused
by some system level crashes when I worked in another project; I tried in
another Mac machine, the display is not so bad but the font still pick up the
wrong [System Default].

Comment 11

[rbs]

Don't worry too much about the '[Sytem Default]'. Internally, it is just a
special blank "". Externally, it is just a label to say to the user that the
back-end will pick a default fallback.

Any chance that this is a dup of bug 188429 instead?

Comment 12

[Yuying Long]

No it's not dup of bug 188429(Mac).  This one is for regrssion [System Default]
name appears on All platforms.

Comment 13

[rbs]

OK, so let's look at it separately:

1) re: _Actual_ rendering:
   It is OK now (per comment 10), i.e., the same _actual_ rendering as older
   builds. (It was some other system level problem.)

2) re: _Appearance_ on the Fonts prefs pannel. There is now a '[System Default]',
   whereas what you expected was an exact name for a font on your system. 

So, we should just focus on 2) from now on, right?

Comment 14

[rbs]

Bringing gerv & blizard in the loop. I can't sell '[System Default]' :-)

Comment 15

[Yuying Long]

> So, we should just focus on 2) from now on, right?
For me, it's Yes.

Comment 16

[Jungshik Shin]

ylong,  I've just downloaded the latest nightly and couldn't reproduce it on
Win2k. '[System Default]' appears _only_ for those entries without the
factory-default setting. For instance, winpref.js (as it's shipped)   doesn't
set 'cursive' and 'fantasy' fonts for TC,SC and KO (and JA, either I guess) and
Mozilla shows me '[System Default]' for them as it should. For others, the
factory-default values(or values I set previously) come up.

I noticed that macpref.js is a bit different. For cursive and fantasy generic
fonts, it has "XXX.cursive" and "XXX.fantasy" for all langGroups. How does
GFX-Mac handle these special values? 

http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/mac/macprefs.js#77

> 05-20-08 and 05-21-03.

  This was when rbs' font-pref change was landed, but why can't we reproduce the
symptom? I'm going to try the latest nightly on Linux.

Comment 17

[rbs]

C.f. earlier comments. There are actually no rendering regressions. It was just
an unfortunate coincidence with some other problem.

The issue right now is just that people are not used to the newish '[System
Default]' and it is easy to point the finger at it when glitches arise. It would
perhaps be simpler and re-assuring to figure out what is the actual first
default font and select it in the UI. I am ruminating about adding an API in the
font enumerator so that the various platforms can return that information.

Comment 18

[Jungshik Shin]

I thought that even when the factory-default values are NOT empty(serif,
sans-serif), ylong had '[system default]'. If he had '[system default]' ONLY for
generic fonts with empty factory-default values (cursive, fantasy), it's working
as designed (although maybe a little bit unexpectedly to end-users as rbs
wrote). As such, the summary line may as well have to be changed to something
gentler :-) with [RFE] tag. 

Comment 19

[Yuying Long]

Yes, on my Mac, it used pick up [System Default] for all my Japanese fonts
except monospace, I realized it was corrupted by some system problem.  After I
re-installed OS, the font render looks OK.

Update the summary...

Comment 20

[rbs]

Attachment: possible middle-ground patch

Another thing is that if the factory defaults are fonts not actually installed
on the user system, it goes back to [System Default]. I now have a patch to dig
out the first font that the back-end will try. Neither approach tells the full
story (that there is an additional fallback list in all cases), but perhpas we
may keep the UI simple to re-assure the user and avoid undue suspicion. Here is
a possible middle-ground patch.

Comment 21

[rbs]

Attachment: WIP - try to figure out the first default font on Xft

It is a little bit subtle than it seems. Since the lists are sorted, the first
default font isn't necessarily the first font in the list. In this WIP, I added
a parameter to EnumerateFonts() so that it can return the index of the
suggested default font within the sorted list. Platforms other than Xft are
just returning 0 since their pref settings are good enough to override. This is
a WIP so that Xft people can hopefully take on.

Comment 22

[rbs]

Comment on attachment 124126 [details] [diff] [review]
WIP - try to figure out the first default font on Xft

+      if (match_pattern != pat)
+	 FcPatternDestroy(result_pattern);


s/result_pattern/match_pattern/

Comment 23

[Christopher Blizzard (:blizzard)]

So are people just upset that there's a [System Default] setting, period? 
People are describing multiple problems in this bug.

I don't want to end up in a situation where we don't have that concept and try
to "guess" what the font is and then save it in the user's prefs.  I've only
just glanced at the patch in this bug but if I was reading it correctly it
didn't clear the user's pref when [System Default] was selected.  This is at
least important for Xft because there is the idea of a global mapping for
"serif" or "sans-serif" and we should respect those settings.  If we save the
default at one time during a user's session later on if they change the global
mapping Mozilla won't pick up those settings, and that's not intended.

Would it be reasonable to have something like:

[System Default (Luxi Serif)]

That both indicated that it was using the system default and what the default
was?  We might want to clear it up a little bit, but I think this reflects what
is accurately happening.  I think that what I've described is kind of ugly, but
we can find a good way of expressing it in the UI.

Also, if you do want to find out the default, I would add an API interface to do
it instead of overloading EnumFonts.  Cleaner that way, I think.

What does everyone think?

Comment 24

[rbs]

>That both indicated that it was using the system default and what the default
>was?

Iterating, what about:

Luxi Serif (Default)

So that
1) there is only one menuitem for any one font;
2) the menuitem only gets flagged as the system default if said so (by the
|get-default-font| API);
3) the UI retains the clearing of the pref for that case.

Since Xft is going to be the only one to implement the API to find out the
default, it will only show/affect Xft. And hopefully everybody will be happy again.

Comment 25

[rbs]

Attachment: patch with the system default in the format 'font-family (Default)'

patch so that the Xft stub for getDefaultFont() can be supported. feedback
welcome as usual.

Comment 26

[rbs]

Attachment: screenshot - possible options

Another possibility is just to make the default font appear in bold in the drop

down font list (common with mail items). The attached screenshot shows the
options suggested so far: (see also secreenshot for [System Default]
http://bugzilla.mozilla.org/attachment.cgi?id=122347&action=view)

1) [System Default]		 - unexpected to users (triggered this bug)
2) [System Default (Luxi Serif)] - (no screenshot) - similar to #1
3) Default (Luxi Serif) 	 - slight variation of #1 & #2
4) Luxi Serif (Default) 	 - buried in the font list, hard to notice
5) <b>Luxi Serif</b>		 - my current favorite because it is simple

blizzard, the final call is yours... which one do you go for?
Since other platforms don't have the notion of a system default for the CSS
generic family, I am simply going to confine your intended effect to Xft at the
end.

Comment 27

[Christopher Blizzard (:blizzard)]

I prefer this one:

3) Default (Luxi Serif) 	 - slight variation of #1 & #2

since it fits the rules of self-description and describes a good system image. 
I don't think using <b>Luxi Serif</b> gives enough context to users.

Is it going to be a problem that users can't reset the fonts to the "default"
for other platforms?

Comment 28

[rbs]

Attachment: patch to show 'Default (fontname)' on platforms where applicable

This patch implements "Default (fontname)" as seen on the last image on
attachment 124259 [details].

>Is it going to be a problem that users can't reset the fonts to the "default"
>for other platforms?

I guess it is not going to be a problem (witness this bug). However, I made the
patch such that platforms that wish to have the "default" menuitem just have to
implement GetDefaultFont(). Also, the format of the label is configurable, in
case a change is needed later.

Comment 29

[rbs]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

seeking r/sr so that I can take the bug off my back, thanks.

Comment 30

[Simon Montagu :smontagu]

Reassigning to rbs since he has a fix.

Comment 31

[Christopher Blizzard (:blizzard)]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

I guess the idea is that the xft code will get flushed out a bit later?

Comment 32

[Kevin McCluskey (gone)]

adt: Since rbs already has a sr and is just waiting for a r I would guess an ETA
of 5/29)

rbs: please change the ETA if this is unreasonable. Thanks.

Comment 33

[rbs]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

jshin, care to r=?

Yeah, the Xft C++ stub is intended to be finished up later. [Something that you
Xft folks might do more easily if this patch goes in.]

Comment 34

[Jungshik Shin]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

r=jshin.
Sorry for the delay.

Comment 35

[rbs]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

Checked in the trunk. Seeking a= for 1.4final branch.

Comment 36

[Asa Dotzler [:asa]]

Comment on attachment 124327 [details] [diff] [review]
patch to show 'Default  (fontname)' on platforms where applicable

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 37

[rbs]

Checked in the 1.4 branch.

Comment 38

[Yuying Long]

Verified fixed in 05-30 1.4 branch build on WinXP.