Closed Bug 109176 Opened 19 years ago Closed 15 years ago

Japanese hiragino text slided up within big button/drop-menu/...

Categories

(Core Graveyard :: GFX: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8.1

People

(Reporter: yinglinxia, Assigned: hsaito54)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.8.0.1, fixed1.8.1, intl)

Attachments

(2 files, 2 obsolete files)

In Japanese build, on all button, drop-menu, radio/check box, ... the area 
size is too big and the Japanese text is slided up. See screenshot below.

This only happens when change the app vers to Japan. It won't happen if keep 
the vers as English. However, we have to change vers to Japan, for system 
dialogs, copy/paste, ...
Attached image screenshot
QA Contact: aselimovic → teruko
And this only happens on OS X.
OS 8/9 build with same files, is fine on OS 8/9.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Component: Bosnian/bs-BA → Internationalization
Product: Browser Localizations → Browser
Target Milestone: --- → mozilla0.9.8
Version: unspecified → other
mass move unimportant m9.8 bug to m9.9 for now. 
Target Milestone: mozilla0.9.8 → mozilla0.9.9
give this to nhotta. remove target
Assignee: ftang → nhotta
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → ---
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2
Target Milestone: mozilla1.2alpha → mozilla1.2beta
I put a break point at nsFontMetricsMac::Init and see what kind of difference we
have regarding font info between different system default languages. I tried two
case "English" and "Japanese", numbers are in device coordinate.

English:
font - "Lucida Grande"
size 11, ascent 11, descent 2, leading 0
size 13, ascent 13, descent 3, leading 0


Japanese:
font - "HiraginoKakuGo Pro-W3"
size 11, ascent 10, descent 1, leading 6
size 13, ascent 11, descent 3, leading 7


A noticeable difference is that Japanese font has leading larger than 0.
So when we include leading for height calculation, Japanese font height is
larger than English because of leading.

In nsFontMetricsMac, we get height including leading.
http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsFontMetricsMac.cpp#104
104   mEmHeight   = mEmAscent + mEmDescent;
105 
106   mMaxHeight  = mEmHeight + mLeading;

Then mMaxHeight is used for line height too.
311 NS_IMETHODIMP nsFontMetricsMac :: GetHeight(nscoord &aHeight)
312 {
313   aHeight = mMaxHeight;
314   return NS_OK;
315 }
316 
317 NS_IMETHODIMP nsFontMetricsMac :: GetNormalLineHeight(nscoord &aHeight)
318 {
319   aHeight = mMaxHeight; // on Windows, it's mEmHeight + mLeading (=
mMaxHeight on the Mac)
320   return NS_OK;
321 }


In the layout, we use ascent as a baseline (but no leading included).

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsTextBoxFrame.cpp#409
409     fontMet->GetMaxAscent(baseline);


In case of Japanese font, the text box height is larger because of the leading
but baseline does not include leading, and the result we see is the charactes
are drawn shifted up as shown in the screen shot.


I dumped font info of other fonts installed on my machine and found those 
new Japanese fonts for MacOSX have large leading values.
One way to workaround the problem is to use other JA fonts (e.g. Osaka) 
for wigets. In fact, I don't know how we get "HiraginoKakuGo Pro-W3" as a 
widget font instead of "Osaka".

fontInfo of Japanese font at size 12 pt.
id= -29602 fontName= ÉqÉâÉMÉmä€ÉS Pro W4 (HiraginoMaruGo W4)
size= 12, ascent= 11, descent= 1, widMax= 12, leading= 6
id= -896 fontName= ÉqÉâÉMÉmñæí© Pro W3 (Hiragino Mincho W3)
size= 12, ascent= 11, descent= 1, widMax= 12, leading= 6
id= -899 fontName= ÉqÉâÉMÉmñæí© Pro W6 (Hiragino Mincho W6)
size= 12, ascent= 11, descent= 1, widMax= 13, leading= 6
id= -17370 fontName= ÉqÉâÉMÉmäpÉS Pro W3 (Hiragino KakuGo W3)
size= 12, ascent= 11, descent= 1, widMax= 12, leading= 6
id= -17373 fontName= ÉqÉâÉMÉmäpÉS Pro W6 (Hiragino KakuGo W6)
size= 12, ascent= 11, descent= 1, widMax= 13, leading= 6
id= -11512 fontName= ÉqÉâÉMÉmäpÉS Std W8 (Hiragino KakuGo W8)
size= 12, ascent= 11, descent= 1, widMax= 14, leading= 6


id= 16384 fontName= Osaka
size= 12, ascent= 12, descent= 3, widMax= 13, leading= 2
id= 16436 fontName= OsakaÅ|ìôïù (Osaka Tohaba)
size= 12, ascent= 10, descent= 2, widMax= 13, leading= 0

id= 16700 fontName= ï?ê¨ñæí© (Heisei Mincho)
size= 12, ascent= 10, descent= 2, widMax= 13, leading= 3
id= 16701 fontName= ï?ê¨äpÉSÉVÉbÉN (Heisei KakuGothic)
size= 12, ascent= 10, descent= 2, widMax= 13, leading= 3
Keywords: intl
Might be related to bug 167236
I applied the patch of bug 167236 but the problem still exists. I think the
patch is specific to form button.
I and Shanjian looked at Apple's font metric info in inside mac.
http://developer.apple.com/techpubs/mac/Text/Text-186.html#HEADING186-0

From the figure, ascent includes internal leading. And their leading is for
external leading.

The problem we have for the widgets (see the screen shot), I think we don't want
to include (external) leading to draw text in widget. The widget text is mostly
a single line and including external leading could cause the glyph drawn shifted
up for fonts with large leading value like Hiragino family.


Target Milestone: mozilla1.2beta → mozilla1.3alpha
*** Bug 142628 has been marked as a duplicate of this bug. ***
Summary: Japanese text slided up within big button/drop-menu/... → Japanese hiragino text slided up within big button/drop-menu/...
Blocks: 210167
For the textmetrics on Windows, the mMaxAscent has an internal leading as
follows. However, for the textmetrics on mac, the mMaxAscent does not have any
internal leading. I think that the baseline of a font with internal Redding is
shifted upwards.

font height = ascent + descent + internal leading

    --------
	|	  internal leading
font   ----
height	|	  ascent
	|
    ----|------------ baseline
	|	  descent
       ----

For the textmetrics on Windows:
mEmHeight = ascent + descent
mEmAscent = ascent
mEmDescent = descent

mMaxHeight = ascent + descent + internal leading
mMaxAscent = ascent + internal leading
mMaxDescent = descent


For the textmetrics on mac:
mEmHeight = ascent + descent
mEmAscent = ascent
mEmDescent = descent

mMaxHeight = ascent + descent + leading
mMaxAscent = ascent
mMaxDescent = descent
Attachment #166063 - Attachment description: patch for adding internal leading → patch for adding internal leading
Comment on attachment 166063 [details] [diff] [review]
patch for adding internal leading

Although I can not test this patch since I do not own mac, our estimators said
that the improvement is clear since the baseline shifts down.
Attachment #166063 - Flags: review?(rbs)
rbs, do you know why |mMaxAscent| of textmetrics on mac does not have an
internal leading? if you do not know the reason, please let me know a reviewer
who can review the code for mac.
Attachment #166063 - Attachment is obsolete: true
Attachment #166063 - Flags: review?(rbs)
Attached patch patch (obsolete) — Splinter Review
Please refer following document for mac's font metrics.
http://developer.apple.com/documentation/Cocoa/Conceptual/FontHandling/index.html

Figure 1 indicats that leading is line gap, and please refer to following document too.
http://developer.apple.com/documentation/mac/Text/Text-145.html#MARKER-9-250
leading
    The measurement in pixels from the descent line to the ascent
    line below it.

I think that mac's leading is a space between lines, but not internal leading.

I wander if a limitation needs as follows, however document of mac's font metrics concerns with all fonts on mac.

-  mLeading    = NSToCoordRound(float(fInfo.leading) * dev2app);
+  if (0 == nsCRT::strcmp(aLangGroup,  "ja"))
+    mLeading  = 0;
+  else
+    mLeading  = NSToCoordRound(float(fInfo.leading) * dev2app);
Attachment #204209 - Flags: review?(rbs)
> http://developer.apple.com/documentation/Cocoa/Conceptual/FontHandling/index.html

Please click "Getting Font Metrics" to see Figure 1.
Assignee: nhottanscp → saito
Status: ASSIGNED → NEW
Component: Internationalization → GFX: Mac
Target Milestone: mozilla1.3alpha → mozilla1.9alpha
Attachment #204209 - Attachment is obsolete: true
Attachment #204209 - Flags: review?(rbs)
Attached patch patch2Splinter Review
I think that Mac's leading is external leading and this value should be used for normal line height.
Attachment #204283 - Flags: review?(rbs)
Our estimator tested patch2 and attached following screenshot. It seems that text position is suitable in the center of each box.
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=3001&action=view
Status: NEW → ASSIGNED
Ash-san,
Please introduce a reviewer who can review patches attached in this bug.
Attachment #204283 - Flags: review?(rbs)
Comment on attachment 204283 [details] [diff] [review]
patch2

roc, could you review this patch?

I think that mac's leading of font metric is external leading, then |mMaxHeight| should not include it and the leading should be used for normal font height.
Attachment #204283 - Flags: superreview?(roc)
Attachment #204283 - Flags: review?(roc)
Comment on attachment 204283 [details] [diff] [review]
patch2

Josh needs to look at this.
Attachment #204283 - Flags: superreview?(roc)
Attachment #204283 - Flags: superreview+
Attachment #204283 - Flags: review?(roc)
Attachment #204283 - Flags: review?(joshmoz)
Comment on attachment 204283 [details] [diff] [review]
patch2

Looks good. I'm not a font expert, but from what I got by reading through and testing this patch, it seems fine.
Attachment #204283 - Flags: review?(joshmoz) → review+
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Comment on attachment 204283 [details] [diff] [review]
patch2

When the font currently used has a leading value on a Mac, this bug is a pretty serious issue related about the display of a text. I think that this patch is low-risk.
Attachment #204283 - Flags: approval1.8.0.1?
Masayuki, please check into the trunk.
(In reply to comment #23)
> Masayuki, please check into the trunk.

O.K. but I cannot check-in now. Please wait until tonight(JST).
checked-in to Trunk.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 233752
Thank you very much. This works very well. I've tested this on most of the sites I designed, and now get the expected display of Hiragino fonts without need for the 'Lucida Grande' hacks.


Hopefully this can land on the branch as well.
This patch does seem reasonable; there's a question as to whether we actually want to take changes that affect the display of Web pages for dot releases.

It seems like this patch would change the height of backgrounds on inline elements but would not change line spacing, so it's not too dangerous.  (Anything that changes line spacing could easily break poorly-designed web pages, which we don't want to do in a security release since it would discourage people from getting security fixes.  But that looks like it's not the case here.)
I think this is an important fix for Japanese users. I would recommend taking it for Firefox 1.5.0.1. I agree with dbaron, this patch does not change line spacing and as such it is not so dangerous.
Flags: blocking1.8.0.1? → blocking1.8.0.1-
Comment on attachment 204283 [details] [diff] [review]
patch2

a=dveditz
Please add fixed1.8.0.1 keyword when checked in.
Attachment #204283 - Flags: approval1.8.1+
Attachment #204283 - Flags: approval1.8.0.1?
Attachment #204283 - Flags: approval1.8.0.1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1-
Flags: blocking1.8.0.1+
Landed on MOZILLA_1_8_0_BRANCH and MOZILLA_1_8_BRANCH
Target Milestone: mozilla1.9alpha → mozilla1.8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.