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

RESOLVED FIXED in mozilla1.8.1

Status

RESOLVED FIXED
17 years ago
10 years ago

People

(Reporter: yinglinxia, Assigned: hsaito54)

Tracking

(Blocks: 1 bug, {fixed1.8.0.1, fixed1.8.1, intl})

Trunk
mozilla1.8.1
PowerPC
Mac OS X
fixed1.8.0.1, fixed1.8.1, intl
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
blocking1.8.0.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

17 years ago
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, ...
(Reporter)

Comment 1

17 years ago
Created attachment 57126 [details]
screenshot
(Reporter)

Updated

17 years ago
QA Contact: aselimovic → teruko
(Reporter)

Comment 2

17 years ago
And this only happens on OS X.
OS 8/9 build with same files, is fine on OS 8/9.

Updated

17 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Updated

17 years ago
Component: Bosnian/bs-BA → Internationalization
Product: Browser Localizations → Browser
Target Milestone: --- → mozilla0.9.8
Version: unspecified → other

Comment 3

17 years ago
mass move unimportant m9.8 bug to m9.9 for now. 
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Comment 4

17 years ago
give this to nhotta. remove target
Assignee: ftang → nhotta
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → ---

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2

Updated

16 years ago
Target Milestone: mozilla1.2alpha → mozilla1.2beta

Comment 5

16 years ago
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.


Comment 6

16 years ago
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

Comment 7

16 years ago
Might be related to bug 167236

Comment 8

16 years ago
I applied the patch of bug 167236 but the problem still exists. I think the
patch is specific to form button.

Comment 9

16 years ago
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.


Updated

16 years ago
Target Milestone: mozilla1.2beta → mozilla1.3alpha

Comment 10

15 years ago
*** Bug 142628 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Summary: Japanese text slided up within big button/drop-menu/... → Japanese hiragino text slided up within big button/drop-menu/...

Updated

15 years ago
Blocks: 210167
(Assignee)

Comment 11

14 years ago
Created attachment 166063 [details] [diff] [review]
patch for adding internal leading

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
(Assignee)

Updated

14 years ago
Attachment #166063 - Attachment description: patch for adding internal leading → patch for adding internal leading
(Assignee)

Comment 12

14 years ago
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)
(Assignee)

Comment 13

14 years ago
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.
(Assignee)

Updated

13 years ago
Attachment #166063 - Attachment is obsolete: true
Attachment #166063 - Flags: review?(rbs)
(Assignee)

Comment 14

13 years ago
Created attachment 204209 [details] [diff] [review]
patch

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)
(Assignee)

Comment 15

13 years ago
> 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
(Assignee)

Updated

13 years ago
Attachment #204209 - Attachment is obsolete: true
Attachment #204209 - Flags: review?(rbs)
(Assignee)

Comment 16

13 years ago
Created attachment 204283 [details] [diff] [review]
patch2

I think that Mac's leading is external leading and this value should be used for normal line height.
Attachment #204283 - Flags: review?(rbs)
(Assignee)

Comment 17

13 years ago
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

Comment 18

13 years ago
Ash-san,
Please introduce a reviewer who can review patches attached in this bug.
(Assignee)

Updated

13 years ago
Attachment #204283 - Flags: review?(rbs)
(Assignee)

Comment 19

13 years ago
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 21

13 years ago
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+

Updated

13 years ago
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
(Assignee)

Comment 22

13 years ago
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?
(Assignee)

Comment 23

13 years ago
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
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Updated

13 years ago
Blocks: 233752

Comment 26

13 years ago
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.)

Comment 28

13 years ago
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+

Comment 30

13 years ago
Landed on MOZILLA_1_8_0_BRANCH and MOZILLA_1_8_BRANCH
Keywords: fixed1.8.0.1, fixed1.8.1
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.