Last Comment Bug 321132 - Japanese font grouping is not correct on font pref dialog
: Japanese font grouping is not correct on font pref dialog
Status: RESOLVED FIXED
[tjp-dl]
: fixed1.8.1, intl, jp-critical, verified1.8.0.2
Product: Core Graveyard
Classification: Graveyard
Component: GFX: Win32 (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla1.8.1
Assigned To: Masatoshi Kimura [:emk]
: Hixie (not reading bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-12-21 11:22 PST by Masatoshi Kimura [:emk]
Modified: 2009-01-22 10:17 PST (History)
2 users (show)
dveditz: blocking1.8.0.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch rv1.0 (1.55 KB, patch)
2005-12-21 11:23 PST, Masatoshi Kimura [:emk]
VYV03354: review-
VYV03354: superreview-
Details | Diff | Splinter Review
Patch rv1.1: more readable (1.66 KB, patch)
2005-12-22 03:33 PST, Masatoshi Kimura [:emk]
rbs: review+
rbs: superreview+
Details | Diff | Splinter Review
Patch rv1.2 (1.66 KB, patch)
2005-12-22 12:33 PST, Masatoshi Kimura [:emk]
VYV03354: review+
VYV03354: superreview+
rbs: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2+
Details | Diff | Splinter Review

Description Masatoshi Kimura [:emk] 2005-12-21 11:22:58 PST
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] 2005-12-21 11:23:43 PST
Created attachment 206526 [details] [diff] [review]
Patch rv1.0
Comment 2 rbs 2005-12-21 20:42:12 PST
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] 2005-12-22 03:32:53 PST
> 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] 2005-12-22 03:33:45 PST
Created attachment 206586 [details] [diff] [review]
Patch rv1.1: more readable
Comment 5 rbs 2005-12-22 12:15:52 PST
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] 2005-12-22 12:33:50 PST
Created attachment 206633 [details] [diff] [review]
Patch rv1.2
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2005-12-23 07:16:15 PST
checked-in.
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2006-02-07 06:38:53 PST
checked-in to 1.8 branch.
Comment 9 Daniel Veditz [:dveditz] 2006-02-16 12:23:24 PST
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] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2006-02-17 07:42:20 PST
checked-in to 1.8.0 branch too. Thanks.

Note You need to log in before you can comment on or make changes to this bug.