408965 - [10.5] cmaps for fonts with Unicode-platform format-12 cmaps are not read

Status: VERIFIED FIXED

(Core :: Graphics, defect, P2)

Description

[Jjgod Jiang]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007121804 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007121804 Minefield/3.0b3pre

In Mac OS X, STHeiti Light, aka. STXihei is the default font for Simplified Chinese, but at present, in Preferences -> Content -> Font & Colors -> Advanced, there is simply no way to choose the Light weight variant of this font, neither STHeiti Light or STXihei is listed, so users can only choose STHeiti (the bolder variant).

Reproducible: Always

Steps to Reproduce:
1. Open Minefield
2. Choose Menu Item: Minefield -> Preferences
3. Open Content -> Font & Colors -> Advanced
4. Try to choose STHeiti
Actual Results:  
STHeiti Regular is used

Expected Results:  
use STHeiti Light

Please compare to how the font panel in Mac OS X behaves and the result of choosing STHeiti in Safari -> Preferences -> Appearance.

Comment 1

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

So the reason for this is that Safari allows for an arbitrary font face choice, whereas we only list the font families in the list and choose the specific family member based on css styles in use.

I'm not sure if choosing the specific font face would complicate things during font selection; I would think not, since this would just be the font that's used for fallback.  However, I worry that given that the desired font is the "Light" font, that we'd keep going back to the Regular font for any css pages that don't explicitly specify Light.. I'm not sure what WebKit does here, I guess it just doesn't modify the chosen default font if the normal weight is selected?

Comment 2

[Jjgod Jiang]

I think it's possible to match that "Light" font without any dirty hacks. Here is what I got on InitFontList():

(fontinit) family: STHeiti, psname: STXihei, face: Light, apple-weight: 3, css-weight: 2, traits: 01000004
(fontinit) family: STHeiti, psname: STHeiti, face: Regular, apple-weight: 7, css-weight: 6, traits: 01000002

So STHeiti Regular has the NSBoldFontMask and STHeiti Light (STXihei) don't, I think we can make use of this flag more extensively. (At present, gfxQuartzFontCache.mm use weight as the main criteria to do font matching.)

But it's just part of the problem, after some investigation last night, I found some fonts I never set as default font use actually chosen, and even I set zh-CN to be the first language in intl.accepted-languages, Japanese fonts like HiraKakuPro-W3 still got higher precedence.

Comment 3

[Jjgod Jiang]

Attachment: Fix CMAP encoding ID checking when platform ID = 0

I think I just find the reason, fonts like STHeiti and STXihei with platform ID = 0, encoding ID = 10 never gets their CMAP loaded because in gfxFontUtils.cpp, EncodingIDUCS4 is set to 10, but in Unicode platform, this value should be 3. Check http://developer.apple.com/textfonts/TTRefMan/RM06/Chap6name.html about "Unicode platform-specific codes".

Here is a patch to fix this.

Comment 4

[Jjgod Jiang]

Oh, there is a typo in my last comment, cmap in STHeiti and STXihei (LiHei Pro as well) has platformID = 0, encodingID = 3.

Comment 5

[Jjgod Jiang]

Attachment: Use STHeiti to replace Hei as the default font

And we really use use STHeiti instead of Hei as default font in modules/libpref/src/init/all.js. Because STHeiti is the default system font for Simplified Chinese locale, it's pretty much the behavior of WebKit too.

Comment 6

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

Comment on attachment 308656 [details] [diff] [review]
Fix CMAP encoding ID checking when platform ID = 0

Going to bounce you over to Stuart for the other review here, he knows this better than I do.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #5)

> And we really use use STHeiti instead of Hei as default font in
> modules/libpref/src/init/all.js. Because STHeiti is the default system font for
> Simplified Chinese locale, it's pretty much the behavior of WebKit too.

That's reasonable; I think Hei is a hold-over from the Mac OS 9-era font prefs.

I'm unsure of the state of our localized font name recognition at this point (John will know :) ), so we might need to use the localized name in the prefs--though, whatever the situation, we should be consistent throughout the Mac section of the prefs and not have some fonts listed with the localized name and others with the Roman script name ;) 

Comment 8

[John Daggett (:jtd)]

There were no fonts that used this type of format-12 cmap in 10.4.  Under 10.5, these fonts use this format:

Apple Symbols
LiHei Pro
LiSong Pro
STFangsong
STHeiti
STKaiti
STSong
STXihei

In all these cases, this is the only cmap-format provided, so we currently won't render anything in these fonts under 10.5, that's why I'm requesting this as "blocking 1.9".

Comment 9

[John Daggett (:jtd)]

Comment on attachment 308663 [details] [diff] [review]
Use STHeiti to replace Hei as the default font

This seems fine.  I think Masayuki should review this.

Comment 10

[John Daggett (:jtd)]

(In reply to comment #7)

> I'm unsure of the state of our localized font name recognition at this point
> (John will know :) ), so we might need to use the localized name in the
> prefs--though, whatever the situation, we should be consistent throughout the
> Mac section of the prefs and not have some fonts listed with the localized name
> and others with the Roman script name ;) 

Once Masayuki's patch for bug 390901 is checked in, it won't matter which name
is used.  In the long run it's probably easier to use the "canonical" names,
the ones Cocoa provides as the default family names independent of locale.  I
think there's probably less chance for typing errors with these.

To dump a list of these with a debug build:

export NSPR_LOG_MODULES=fontInfoLog:5
export NSPR_LOG_FILE=fontinfo.log

Comment 11

[John Daggett (:jtd)]

(In reply to comment #2)
> But it's just part of the problem, after some investigation last night, I found
> some fonts I never set as default font use actually chosen, and even I set
> zh-CN to be the first language in intl.accepted-languages, Japanese fonts like
> HiraKakuPro-W3 still got higher precedence.

If you find this with the latest trunk builds, please enter another bug for this, we should only use Japanese fonts when a character was not found in a Chinese font.

Comment this line out to enable dumping of text runs, this will also show you the exact font fallback that is occurring:

http://mxr.mozilla.org/mozilla/source/gfx/thebes/src/gfxAtsuiFonts.cpp#65

Comment 12

[John Daggett (:jtd)]

Patch looks fine except for the one small nit below.

+    #define acceptableUCS4Encoding(p, e) \
+        ((platformID == PlatformIDMicrosoft && encodingID == EncodingIDUCS4ForMicrosoftPlatform) || \
+         (platformID == PlatformIDUnicode   && encodingID == EncodingIDUCS4ForUnicodePlatform))

Move this above, just below the other related defines just to keep things together.

Comment 13

[Jjgod Jiang]

T(In reply to comment #12)
> Patch looks fine except for the one small nit below.
> 
> +    #define acceptableUCS4Encoding(p, e) \
> +        ((platformID == PlatformIDMicrosoft && encodingID ==
> EncodingIDUCS4ForMicrosoftPlatform) || \
> +         (platformID == PlatformIDUnicode   && encodingID ==
> EncodingIDUCS4ForUnicodePlatform))
> 
> Move this above, just below the other related defines just to keep things
> together.

Then we should move related enums for PlatformIDMicrosoft, PlatformIDUnicode, etc. above too, is that acceptable?

Comment 14

[John Daggett (:jtd)]

That would be fine with me, I'll let stuart (pavlov) comment on that.

Comment 15

[John Daggett (:jtd)]

Attachment: testcase, simplified chinese with STxxx font styles

On 10.5, all the text will display with the same semi-bolded font.  With the cmap fix, each paragraph will display with the correct named font.

Comment 16

[John Daggett (:jtd)]

Attachment: screenshot, rendering with latest trunk

Comment 17

[John Daggett (:jtd)]

Attachment: screenshot, rendering with cmap patch

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 308663 [details] [diff] [review]
Use STHeiti to replace Hei as the default font

> -pref("font.name-list.sans-serif.zh-CN", "Hei");
> -pref("font.name-list.monospace.zh-CN", "Hei");
> +pref("font.name-list.sans-serif.zh-CN", "STHeiti");
> +pref("font.name-list.monospace.zh-CN", "STHeiti");

Should name-list also have "Hei"? (i.e., should that be "STHeiti,Hei"?) If it's not so, r=me.

Comment 19

[Jjgod Jiang]

(In reply to comment #18)
> Should name-list also have "Hei"? (i.e., should that be "STHeiti,Hei"?) If it's
> not so, r=me.

None of the XP_MACOSX pref("font.name-list.*) settings in modules/libpref/src/init/all.js has multiple values, so I'm not sure if that's needed. 

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #19)
> (In reply to comment #18)
> > Should name-list also have "Hei"? (i.e., should that be "STHeiti,Hei"?) If it's
> > not so, r=me.
> 
> None of the XP_MACOSX pref("font.name-list.*) settings in
> modules/libpref/src/init/all.js has multiple values, so I'm not sure if that's
> needed. 

If STHeiti has all characters which is contained in Hei, you don't need to add Hei. But if not so, Hei will be used for such characters if you add Hei to the name-list.

Comment 21

[John Daggett (:jtd)]

(In reply to comment #18)
> Should name-list also have "Hei"? (i.e., should that be "STHeiti,Hei"?) If it's
> not so, r=me.

The set of codepoints covered by STHeiti completely covers the codepoints in the Hei cmap, so this isn't necessary.  Assuming the glyphs used by STHeiti are more appropriate, I think we should take this change.

ftxdumperfuser -t cmap /System/Library/Fonts/Hei.dfont

Note: we use the format-4 table on 10.4, the cmap subtables in other formats don't apply.

Comment 22

[John Daggett (:jtd)]

Comment on attachment 308663 [details] [diff] [review]
Use STHeiti to replace Hei as the default font

I think we should try to get this in for B5.

Comment 23

[John Daggett (:jtd)]

Attachment: patch, v.0.2, Fix CMAP encoding ID checking when platform ID = 0, slight cleanup

Comment 24

[Stuart Parmenter]

Comment on attachment 308663 [details] [diff] [review]
Use STHeiti to replace Hei as the default font

seems ok?

Comment 25

[John Daggett (:jtd)]

both patches checked in.

Comment 26

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050621 Firefox/3.0pre and the testcase from John -> Build has expected result

--> Verified fixed