129188 - [Mac OS X]The western font looks ugly when locale set to Chinese or Korean

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Yuying Long]

Build: 03-05 build

When setting the system locale to Chinese (both SC and TC) or Korean, the
western font looks ugly, the whole UI for the application doesn't look good.

Screen shots for Japanese and TradChinese locale will be followed, you will see
the western fonts with Japanese locale are looks fine, but the other one is not
good, and they are using same fonts.

Comment 1

[Yuying Long]

Attachment: screen shot when locale set to TradChinese

Comment 2

[Yuying Long]

Attachment: screen shot when locale set to Japanese

Comment 3

[Yuying Long]

On same locale setting with IE5.1-US, the western fonts are displayed fine, also
seems it looks OK with textEdit.

-> nsbeta1.

Comment 4

[Roy Yokoyama]

MacOS X
->nhotta

Comment 5

[Frank Tang]

nsbeta1+ , give to ftang

Comment 6

[Frank Tang]

assign

Comment 7

[Frank Tang]

It may caused by the following code
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsDeviceContextMac.cpp#322
242 NS_IMETHODIMP nsDeviceContextMac :: GetSystemFont(nsSystemFontID aID, nsFont
*aFont) const


319 Str255 fontName;
320 SInt16 fontSize;
321 Style fontStyle;
322 ::GetThemeFont(fontID, smSystemScript, fontName, &fontSize, &fontStyle);
323 fontName[fontName[0]+1] = 0;
324 aFont->name.AssignWithConversion( (char*)&fontName[1] );

also 
336 aFont->name.Assign(NS_LITERAL_STRING("geneva"));

the code looks like it assume the fontName is in ASCII but we all know it is not
always true. 

Comment 8

[Frank Tang]

ok,what happen is this, the nsDeviceContextMac :: GetSystemFont

will ask the apperance manager to return a font for the system script. Somehow
the Trad Chinese return a very bad looking one. 
work around is always ask the Roman script one. 

Comment 9

[Frank Tang]

Attachment: v1 of patch

Comment 10

[Frank Tang]

nhotta said this is not good enough, he said if the Chinese one is bad then we
should fix only the chinese one. 

Comment 11

[Frank Tang]

the problem is I see two issue here.
1. the Japanese one GetThemeFont return are corrupted by the
AssignWithConversion method (instead of convert Shift_JIS to Unicode), and
therefore the Japanese theme font may not be used at all. (and that is why it
does not show the ugly looking glyph)
2. the quality of ASCII text is very bad with Chinese and Korean themem font.

Comment 12

[Frank Tang]

The reason the Japanese looks ok is because
 aFont->name.AssignWithConversion( (char*)&fontName[1] );

damange the Japanese font name and therefore it does not use Japanese font. Once
we fix that part, the problem also show up in Japanese.So. I still suggest we
use the smRoman font.

Comment 13

[Frank Tang]

Attachment: v2 of the patch

Comment 14

[Frank Tang]

this patch fix two issue.
1. the 2nd part convert the font name correctly into unicode
2. the 1st part force to use smRoman since we know the ASCII part in other
script code render ugly.

Comment 15

[nhottanscp]

Comment on attachment 73547 [details] [diff] [review]
v2 of the patch

* fix the tab
* '\0' - cast to PRUnichar

* Only use smRoman for the scripts which we have the problem, Korean,
Simplified Chinese, Traditional Chinese but not for Japanese.

+	 ::GetThemeFont(fontID, smRoman, fontName, &fontSize, &fontStyle);

Comment 16

[Frank Tang]

fixed and checkin 

Comment 17

[Frank Tang]

sorry, close the wrong bug, reopen

Comment 18

[Frank Tang]

Attachment: patch v3

Comment 19

[nhottanscp]

Comment on attachment 73608 [details] [diff] [review]
patch v3

+	    aFont->name.AssignWithConversion( (char*)&fontName[1] );

This also exists in the original code but it has to pass a length fontName[0].

Please put a comment for the reason of assigning smRoman for smKorean,
smTradChinese and smSimpChinese.

Comment 20

[Simon Fraser [no longer active]]

Address nhotta's comments, and sr=sfraser

Comment 21

[Frank Tang]

Attachment: patch v4 adderss nhotta's comment

Comment 22

[nhottanscp]

Comment on attachment 74165 [details] [diff] [review]
patch v4 adderss nhotta's comment

r=nhotta

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 74165 [details] [diff] [review]
patch v4 adderss nhotta's comment

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 24

[Frank Tang]

fixed and check into trunk

Comment 25

[Simon Fraser [no longer active]]

Belated sr comment: we need to factor that TEC converter stuff into a single
place in gfx. We have that 20-odd lines of code in several places.

Comment 26

[Yuying Long]

Fixed verified on 03-18 trunk build/Mac 10.1.3.