1096800 - Default sans-serif font for zh-CN on windows is mapped to SimSun, which is actually a serif font

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Duan Yao]

Attachment: font-test-serif-sans.html

User Agent: Mozilla/5.0 (Windows NT 6.3; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141106201515

Steps to reproduce:

Open "about:config" on a windows with zh-CN locale, check the value of "font.name.sans-serif.zh-CN".



Actual results:

Default value of "font.name.sans-serif.zh-CN" is "SimSun", which is same as "font.name.serif.zh-CN".

"SimSun"(or "宋体" in Chinese) is a Chinese font which is regarded as serif font, so the default is definitely wrong.

As a result, on firefox on windows with zh-CN locale, texts with "font-family: sans-serif" style are renderred same as "font-family: serif", as shown by the attachment.



Expected results:

"font.name.sans-serif.zh-CN" should be mapped to a sans-serif font by default, such as "SimHei"(or "黑体" in Chinese), or "Microsoft Yahei"(or "微软雅黑" in Chinese). With this setting, texts with "font-family: sans-serif" look correct.

Comment 1

[Duan Yao]

This bug exists at least since FF 29.
IE 11 do it correctly, see the attached screenshots.

Comment 2

[Duan Yao]

Attachment: screenshot of firefox renderring sans-serif/serif on zh-CN windows

Comment 3

[Duan Yao]

Attachment: screenshot of IE11 renderring sans-serif/serif on zh-CN windows

Comment 4

[John Daggett (:jtd)]

Current defaults for Windows zh-CN:
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#2700

pref("font.name.serif.zh-CN", "SimSun");
pref("font.name.sans-serif.zh-CN", "SimSun");
pref("font.name.monospace.zh-CN", "SimSun");
pref("font.name-list.serif.zh-CN", "MS Song, SimSun, SimSun-ExtB");
pref("font.name-list.sans-serif.zh-CN", "MS Song, SimSun, SimSun-ExtB");
pref("font.name-list.monospace.zh-CN", "MS Song, SimSun, SimSun-ExtB");

Need to figure out the right fonts here, including both for Win7+ and for Win XP.

It would be useful to know:

- what are the ideal sans-serif fonts for WinXP/Win7/Win8?
- what is IE11 using?
- what did XP versions of IE use?

We can then include the best default in the font.name prefs and add the best-case fallbacks to the font.name-list prefs.

Comment 5

[Duan Yao]

IE 11 on windows 8.1 for zh-CN seems using "Microsoft Yahei"(or "微软雅黑" in Chinese) font as sans-serif for Chinese scripts, and "SimSun"(or "宋体" in Chinese) as serif. For western scripts, IE 11 mostly use Arial and Times New Roman. 
See "Default font changes" http://msdn.microsoft.com/en-us/library/ie/dn467844%28v=vs.85%29.aspx

"Microsoft Yahei" was shipped since windows vista, "SimHei" since windows 2000, and "SimSun" since windows 95. 
See http://en.wikipedia.org/wiki/List_of_CJK_fonts

So I think FF can use "Microsoft Yahei"/"SimSun" on windows vista and above, "SimHei"/"SimSun" on XP.

Comment 6

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Available Chinese fonts in Windows

Based on Duan Yao's helpful info in comment 5, especially 
https://msdn.microsoft.com/en-us/library/ie/dn467844(v=vs.85).aspx, and further research from MS's Fonts supplied with Windows * pages
http://www.microsoft.com/typography/fonts/product.aspx?PID=135
http://www.microsoft.com/typography/fonts/product.aspx?PID=145
http://www.microsoft.com/typography/fonts/product.aspx?PID=149
http://www.microsoft.com/typography/fonts/product.aspx?PID=161
http://www.microsoft.com/typography/fonts/product.aspx?PID=164
I've assembled this table of available fonts and their properties.
Based on these data I've come to the same conclusion as Duan Yao, and will shortly attach a proposed patch implementing such.
I've also included setting the font.name.cursive.zh-CN pref to KaiTi, since it seems an appropriate choice.

Comment 7

[Ian Moody [:Kwan] (UTC+1)]

Attachment: proposed changes

Comment 8

[John Daggett (:jtd)]

Comment on attachment 8559469 [details] [diff] [review]
proposed changes

Review of attachment 8559469 [details] [diff] [review]:
-----------------------------------------------------------------

seems reasonable

Comment 9

[John Daggett (:jtd)]

trybuild:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bb6f37e8f30

Comment 10

[John Daggett (:jtd)]

Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96de29a78170

Comment 11

[Carsten Book [:Tomcat]]

sorry had to back this out for xperf perma failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6458765&repo=mozilla-inbound starting with this push

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

This is the error we see:
    TEST-UNEXPECTED-FAIL | mainthreadio | File '{fonts}\msyh.ttf' was accessed and we were not expecting it: {'Count': 1, 'Duration': 0.033023, 'RunCount': 1} 

xperf looks at all files access on the mainthread and reports unknown ones.  This is our whitelist of files:
http://hg.mozilla.org/build/talos/file/60d6980dec28/talos/mtio-whitelist.json

Most likely a font is something we can just add to the whitelist.

This is fairly easy to do, we can adjust it in the talos repo and update talos.json to point to the latest revision in that repo.  Before we do that, we need to confirm this is what is expected.

Comment 13

[Ian Moody [:Kwan] (UTC+1)]

(In reply to Joel Maher (:jmaher) from comment #12)
> This is the error we see:
>     TEST-UNEXPECTED-FAIL | mainthreadio | File '{fonts}\msyh.ttf' was
> accessed and we were not expecting it: {'Count': 1, 'Duration': 0.033023,
> 'RunCount': 1} 
> 
> xperf looks at all files access on the mainthread and reports unknown ones. 
> This is our whitelist of files:
> http://hg.mozilla.org/build/talos/file/60d6980dec28/talos/mtio-whitelist.json
> 
> Most likely a font is something we can just add to the whitelist.
> 
> This is fairly easy to do, we can adjust it in the talos repo and update
> talos.json to point to the latest revision in that repo.  Before we do that,
> we need to confirm this is what is expected.

Ah, in that case I think KaiTi (simkai.ttf) and Microsoft YaHei (msyh.ttf) need whitelisting then, since they are newly added and thus this failure is expected, knowing about the existence of the whitelist.

SimHei (simhei.ttf) and Microsoft YaHei Bold (msyhbd.ttf) already seem to be in there however.

Comment 14

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Add YaHei and KaiTi to the talos whitelist

Here's a patch with (I believe) the needed talos changes.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8562082 [details] [diff] [review]
Add YaHei and KaiTi to the talos whitelist

Review of attachment 8562082 [details] [diff] [review]:
-----------------------------------------------------------------

this patch is correct.  I recommend landing this on the talos repository, then update talos.json:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos.json?from=talos.json&case=true#8

You could land the talos.json updates with the original patch in this bug!

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/3edc0c98940e

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8562082 [details] [diff] [review]
Add YaHei and KaiTi to the talos whitelist

please update testing/talos/talos.json to point to revision:
3edc0c98940e

no need to update the .zip for android.

Comment 18

[Ian Moody [:Kwan] (UTC+1)]

Attachment: patch with talos.json rev change

Carrying review forward

https://treeherder.mozilla.org/#/jobs?repo=try&revision=50f986dda678

Comment 19

[Ian Moody [:Kwan] (UTC+1)]

Try run all green, including the previously failing xperf test that caused the back-out.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/179a122ae672

Comment 21

[John Daggett (:jtd)]

Ok, so my mistake here was not running the 'xperf' test as part of my try push. I'll add that to future try pushes.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/179a122ae672