Closed
Bug 109176
Opened 23 years ago
Closed 19 years ago
Japanese hiragino text slided up within big button/drop-menu/...
Categories
(Core Graveyard :: GFX: Mac, defect)
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)
55.17 KB,
image/jpeg
|
Details | |
1.48 KB,
patch
|
jaas
:
review+
roc
:
superreview+
dveditz
:
approval1.8.0.1+
dveditz
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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•23 years ago
|
||
Reporter | ||
Updated•23 years ago
|
QA Contact: aselimovic → teruko
Reporter | ||
Comment 2•23 years ago
|
||
And this only happens on OS X.
OS 8/9 build with same files, is fine on OS 8/9.
Updated•23 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Updated•23 years ago
|
Component: Bosnian/bs-BA → Internationalization
Product: Browser Localizations → Browser
Target Milestone: --- → mozilla0.9.8
Version: unspecified → other
Comment 3•23 years ago
|
||
mass move unimportant m9.8 bug to m9.9 for now.
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Comment 4•23 years ago
|
||
give this to nhotta. remove target
Assignee: ftang → nhotta
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → ---
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2
Updated•22 years ago
|
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Comment 5•22 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•22 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
Might be related to bug 167236
Comment 8•22 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•22 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•22 years ago
|
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Comment 10•21 years ago
|
||
*** Bug 142628 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
Summary: Japanese text slided up within big button/drop-menu/... → Japanese hiragino text slided up within big button/drop-menu/...
Assignee | ||
Comment 11•20 years ago
|
||
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•20 years ago
|
Attachment #166063 -
Attachment description: patch for adding internal leading → patch for adding internal leading
Assignee | ||
Comment 12•20 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•20 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•19 years ago
|
Attachment #166063 -
Attachment is obsolete: true
Attachment #166063 -
Flags: review?(rbs)
Assignee | ||
Comment 14•19 years ago
|
||
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•19 years ago
|
||
> http://developer.apple.com/documentation/Cocoa/Conceptual/FontHandling/index.html
Please click "Getting Font Metrics" to see Figure 1.
Updated•19 years ago
|
Assignee: nhottanscp → saito
Status: ASSIGNED → NEW
Component: Internationalization → GFX: Mac
Target Milestone: mozilla1.3alpha → mozilla1.9alpha
Assignee | ||
Updated•19 years ago
|
Attachment #204209 -
Attachment is obsolete: true
Attachment #204209 -
Flags: review?(rbs)
Assignee | ||
Comment 16•19 years ago
|
||
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•19 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•19 years ago
|
||
Ash-san,
Please introduce a reviewer who can review patches attached in this bug.
Assignee | ||
Updated•19 years ago
|
Attachment #204283 -
Flags: review?(rbs)
Assignee | ||
Comment 19•19 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•19 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+
Assignee | ||
Comment 22•19 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•19 years ago
|
||
Masayuki, please check into the trunk.
Comment 24•19 years ago
|
||
(In reply to comment #23)
> Masayuki, please check into the trunk.
O.K. but I cannot check-in now. Please wait until tonight(JST).
Comment 25•19 years ago
|
||
checked-in to Trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
![]() |
||
Comment 26•19 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•19 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.
Updated•19 years ago
|
Flags: blocking1.8.0.1? → blocking1.8.0.1-
Comment 29•19 years ago
|
||
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+
Updated•19 years ago
|
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1-
Flags: blocking1.8.0.1+
Comment 30•19 years ago
|
||
Landed on MOZILLA_1_8_0_BRANCH and MOZILLA_1_8_BRANCH
Keywords: fixed1.8.0.1,
fixed1.8.1
Updated•19 years ago
|
Target Milestone: mozilla1.9alpha → mozilla1.8.1
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•