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

RESOLVED FIXED

Status

Core Graveyard
GFX: Gtk
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Jungshik Shin, Assigned: Jungshik Shin)

Tracking

({intl})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

9.34 KB, patch
Brian Ryner (not reading)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
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>
(Assignee)

Comment 1

13 years ago
Created attachment 169722 [details] [diff] [review]
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)
Attachment #169722 - Flags: superreview?(rbs)
Attachment #169722 - Flags: review?(blizzard)

Comment 2

13 years ago
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?
Attachment #169722 - Flags: superreview?(rbs) → superreview-
(Assignee)

Comment 3

13 years ago
(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. 


Summary: [xft] font-name.list (fontset) support → [xft] font-name.list (fontset) support

Comment 4

13 years ago
> 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).
(Assignee)

Comment 5

13 years ago
(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.
(Assignee)

Comment 6

13 years ago
Created attachment 170457 [details] [diff] [review]
updated patch

addressed rbs' concerns
Attachment #169722 - Attachment is obsolete: true
Attachment #170457 - Flags: superreview?(rbs)
Attachment #170457 - Flags: review?(blizzard)

Comment 7

13 years ago
Comment on attachment 170457 [details] [diff] [review]
updated patch

sr=rbs
Attachment #170457 - Flags: superreview?(rbs) → superreview+
(Assignee)

Updated

13 years ago
Attachment #169722 - Flags: review?(blizzard)
(Assignee)

Comment 8

13 years ago
Comment on attachment 170457 [details] [diff] [review]
updated patch

ask bryner for review
Attachment #170457 - Flags: review?(blizzard) → review?(bryner)
Attachment #170457 - Flags: review?(bryner) → review+

Comment 9

13 years ago
Firefox linux is burning because gFontMetricsPSM is only defined if PR_LOGGING
is enabled.
(Assignee)

Comment 10

13 years ago
fixed. build issue was also taken care of.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.