229394 - [xft] font-name.list (fontset) support

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Jungshik Shin]

As demonstrated in bug 227815, font-name.list (fontset) support is quite useful
for some cases. Some non-Western European script users  want this because Latin
letters and numbers in fonts for their native scripts are rather ugly (at least
not as good as glyphs in Western fonts). Therefore, they want Latin letters to
be rendered in a Western font and characters in their native script to be
rendered in a font for their native script. X11 users (especially CJK) are
familiar with this concept (fontset). 

This is similar to an example given in CSS2 specification section 15.2.2. The
difference is that it's for the client-side setting (or can be viewed as the
client-side CSS).

<quote>
 BODY {font-family: Baskerville, "Heisi Mincho W3", Symbol, serif }

The glyphs available in the "Baskerville" font (a font that covers only
Latin characters) will be taken from that font, Japanese glyphs will be
taken from "Heisi Mincho W3", and the mathematical symbol glyphs will come
from "Symbol".  Any others will come from the generic font family 'serif'.
</quote>

Comment 1

[Jungshik Shin]

Attachment: patch

This is yet another step to move Gfx:Xft toward par with Gfx:Win. While I'm at
it, I also got rid of nsIPref (which has been obsolete for almost 4 years by
now)

Comment 2

[rbs]

Comment on attachment 169722 [details] [diff] [review]
patch

1) You might want to move the hyphen count test into the function since you are
using it all the time. That will reduce the code quite a bit and  look nicer.

+	     if (NS_FFRECountHyphens(family) < 3) {
+		 AddFFREandLog(aPattern, family, aLogModule);
+	     }

2) Any reason why you had to skip the font.name-list when font.name isn't set?

Comment 3

[Jungshik Shin]

(In reply to comment #2)
> (From update of attachment 169722 [details] [diff] [review] [edit])
> 1) You might want to move the hyphen count test into the function since you are
> using it all the time. That will reduce the code quite a bit and  look nicer.

Ok.

> 2) Any reason why you had to skip the font.name-list when font.name isn't set?
 Yes, there is although that doesn't have a very solid basis. For Xft, we don't
set the default font.name.*.  Neither do we set the default font.name-list.*.
Instead, we use the OS (actually desktop manager) setting  for font.name.*. I
think that when font.name.* is not set explicitly by a user, it'd be least
surprising to her to skip font.name-list.* altogether. Hmmm. some (advanced)
users may wonder why their setting for font.name-list.* (via 'about:config' or
direct editing of prefs.js)    is not taken into account when font.name.* is
left unset. I'll listen to you, blizzard, bryner and others about this. 

Comment 4

[rbs]

> For Xft, we don't set the default font.name.*.

You meant that the shipped prefs are for X11 Core fonts -- which don't pass that
hyphen count test.

I think skipping font.name-list under this condition might be too clever for its
own good. I will go for keeping it. It will ease giving the same explanation on
all platforms that: if set, it is used, but font.name has precedence (and can be
unset/empty or set to an inexisting font).

Comment 5

[Jungshik Shin]

(In reply to comment #4)
> > For Xft, we don't set the default font.name.*.
> 
> You meant that the shipped prefs are for X11 Core fonts -- which don't pass that
> hyphen count test.

Yes and more. We could have specified the default set of fonts for xft build by
enclosing them with '#ifdef ..TOOLKIT=gtk2' or '#ifdef MOZ_ENABLE_XFT' in
all.js, but we didn't. (blizzard preferd to get them from the desktop setting)

> I think skipping font.name-list under this condition might be too clever for its
> own good. I will go for keeping it. 

All right.

Comment 6

[Jungshik Shin]

Attachment: updated patch

addressed rbs' concerns

Comment 7

[rbs]

Comment on attachment 170457 [details] [diff] [review]
updated patch

sr=rbs

Comment 8

[Jungshik Shin]

Comment on attachment 170457 [details] [diff] [review]
updated patch

ask bryner for review

Comment 9

[Ian Neal]

Firefox linux is burning because gFontMetricsPSM is only defined if PR_LOGGING
is enabled.

Comment 10

[Jungshik Shin]

fixed. build issue was also taken care of.