321132 - Japanese font grouping is not correct on font pref dialog

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Tools-Options-Content-Fonts & Colors-Advanced.
2. Select Japanese from "Fonts for".
3. Open "Serif"/"Sans-serif"/"Monospace" drop-down lists.
Actual result:
"MS Mincho" is listed in the first group of "Serif" drop-down list.
"MS PGothic" and "MS UI Gothic" are listed in the first group of "Monospace" drop-down list.
Expected result:
"MS Mincho" should be listed in the first group of "Monospace" drop-down list.
"MS PGothic" and "MS UI Gothic" should be listed in the first group of "Sans-serif" drop-down list.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.0

Comment 2

[rbs]

This patch is hard to read.

For ease of reference here is what the MSDN doc says:
=========
FF_DONTCARE:
Use default font.

FF_ROMAN:
Fonts with variable stroke width (proportional) and with serifs. MS Serif is an example.

FF_MODERN:
Fonts with constant stroke width (monospace), with or without serifs. Monospace fonts are usually modern. Pica, Elite, and CourierNew are examples.

FF_SWISS:
Fonts with variable stroke width (proportional) and without serifs. MS Sans Serif is an example.

FF_SCRIPT:
Fonts designed to look like handwriting. Script and Cursive are examples.

FF_DECORATIVE:
Novelty fonts. Old English is an example.
=========

So, we can very well have monospace-serif fonts and monospace-sans-serif fonts, but for them to be variable-pitch is odd... It seems that it is the Japanese fonts that are peculiar. (We have seen so far that they seem to have unusual settings/metrics.)

Anyway, my reading of your patch is that what you want is to treat any font as a monospace font if it has the fixed-pitch flag, and treat any monospace-sans-serif font as a sans-serif font.

Can you code exactly what you want outside the |switch|, leaving the switch as a fall through path if you criteria are not met. It will make the code easier to read and check for review.

Comment 3

[Masatoshi Kimura [:emk]]

> For ease of reference here is what the MSDN doc says:
> (snip)
Please notice the word "stroke width". Stroke width doesn't always agree with pitch. We may assume that fonts having constant stroke width are also fixed pitch in Western typography world, but Japanese typography follows the different manner.

"MS Mincho" and "MS PGothic" are only a couple of examples among many.
AFAIK all Japanese Mincho fonts belong to FF_ROMAN even if they are fixed pitch because they have variable stroke width (unlike Courier New).
And all Japanese Gothic fonts belong to FF_MODERN even if they are variable pitch because they have constant stroke width (unlike Arial).

Some documents say that Japanese fonts only use FF_ROMAN(Mincho) or FF_MODERN(Gothic), but I don't quote because unfortunately all of them were written in Japanese :-(

Comment 4

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.1: more readable

Comment 5

[rbs]

Comment on attachment 206586 [details] [diff] [review]
Patch rv1.1: more readable

r+sr=rbs, much better.

+  PRUint8 family = aFont->logFont.lfPitchAndFamily & 0xF0;
+  PRUint8 pitch = aFont->logFont.lfPitchAndFamily & 0xF;

nit: 0x0F gives a nicer symmetry:

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.2

Comment 7

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

checked-in.

Comment 8

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

checked-in to 1.8 branch.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 206633 [details] [diff] [review]
Patch rv1.2

approved for 1.8.0 branch, a=dveditz for drivers

Comment 10

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

checked-in to 1.8.0 branch too. Thanks.