Closed
Bug 425004
Opened 17 years ago
Closed 8 years ago
Text descenders of "g, j, p, q, y" are hidden when using some fonts
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: t.matsuu, Assigned: karlt)
References
(Depends on 2 open bugs)
Details
Attachments
(8 files, 2 obsolete files)
User-Agent: Mozilla/5.0 (X11; U; Linux i686; ja; rv:1.9b5pre) Gecko/2008032504 Minefield/3.0b5pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ja; rv:1.9b5pre) Gecko/2008032504 Minefield/3.0b5pre
If certain fonts are used for desktop or web contents, text descenders of "g, j, p, q, y" are hidden.
As far as I tested, the following fonts affect to this bug.
IPA明朝,IPAMincho
IPA P明朝,IPAPMincho
IPAゴシック,IPAGothic
IPA Pゴシック,IPAPGothic
IPA UIゴシック,IPAUIGothic
IPA X0208 明朝,IPAX0208Mincho
IPA X0208 P明朝,IPAX0208PMincho
IPA X0208 ゴシック,IPAX0208Gothic
IPA X0208 Pゴシック,IPAX0208PGothic
IPA X0208 UIゴシック,IPAX0208UIGothic
available from:
http://ossipedia.ipa.go.jp/ipafont/
(Information only in Japanese is available. But we can redistribute them if no modification is applied.)
The fonts above are higher quality Japanese fonts than ones (ie. Sazanami-mincho, Sazanami-gothic font etc.) which are distributed by most Linux distributors. So some users prefer to use them and should be fixed before released.
Reproducible: Always
Mozilla-gumi bugzilla:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=6083
Screenshot of gedit and Minefield with ja_JP.UTF-8 locale
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=3707
Screenshot of gedit and Minefield with C locale
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=3708
The height of URL bar is 2 dots shorter than the search dialog in gedit when Minefield is run with ja_JP.UTF-8 locale.
Text descenders are shown if the input area is scrolled by using mouse.
Work status:
2008020718 OK
2008021119 NG
bonsai:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-07+18%3A00%3A00&maxdate=2008-02-11+19%3A59%3A59&cvsroot=%2Fcvsroot
Reporter | ||
Updated•17 years ago
|
Flags: blocking-firefox3?
Reporter | ||
Comment 1•17 years ago
|
||
The problem is not occured if firefox is run as the following locales:
LANG=C firefox
LANG=en_US.UTF-8 firefox
LANG=it_IT.UTF-8 firefox
And it is occured if firefox is run as the following locales:
LANG=ja_JP.UTF-8 firefox
LANG=ko_KR.UTF-8 firefox
LANG=zh_CN.UTF-8 firefox
Is the calculation of the font height different between CJK locale and non-CJK ones?
Comment 2•17 years ago
|
||
Comment 3•17 years ago
|
||
font-family: "Bitstream Vera Sans", "IPAPGothic";
is good, but sans-serif (fontconfig switches fonts) is not good.
Comment 4•17 years ago
|
||
This ~/.fonts.conf is used when previous screenshot taken.
This is similar to the style "font-family: "Bitstream Vera Sans", "IPAPGothic";"
but the height of input is different.
Assignee | ||
Updated•17 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•17 years ago
|
Component: General → Layout: Form Controls
Flags: blocking-firefox3?
Product: Firefox → Core
QA Contact: general → layout.form-controls
Assignee | ||
Comment 5•17 years ago
|
||
(Transferring MATSUURA's blocking request from firefox3 to 1.9.)
Flags: blocking1.9?
Comment 6•17 years ago
|
||
Is this actually a form controls issue? Or would a <span> with such text in it also not extend far enough? The latter seems much more likely to me....
Reporter | ||
Comment 7•17 years ago
|
||
Atsushi reported the more refined regression period at Mozilla-gumi bugzilla.
2008021004 is good
and
2008021104 is NOT good.
And he has suggetset this is the regression of the following checkin:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&date=explicit&mindate=2008-02-10+22%3A45&maxdate=2008-02-10+22%3A45&cvsroot=%2Fcvsroot
I have reverted the bonsai checkin above and built. Attachment 317770 [details] shows properly but it is not affected to URL bar.
Reporter | ||
Comment 8•17 years ago
|
||
Comment 9•17 years ago
|
||
Comment 10•17 years ago
|
||
(In reply to comment #6)
> Is this actually a form controls issue? Or would a <span> with such text in it
> also not extend far enough? The latter seems much more likely to me....
outline of <span> is good.
But, background-color of <span> is sometimes narrow.
Updated•17 years ago
|
Attachment #312030 -
Attachment mime type: text/html → text/html; charset=Shift_JIS
Comment 11•17 years ago
|
||
OK, so not a form control issue.
Component: Layout: Form Controls → Layout: Fonts and Text
QA Contact: layout.form-controls → layout.fonts-and-text
I don't think we can block on this. However, Karl can probably look at this if priorities allow.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → mozbugz
Status: NEW → ASSIGNED
Assignee | ||
Comment 14•16 years ago
|
||
This fixes this bug and bug 481751 by using a natural baseline for the text control frames and setting the initial scroll position of the content so that the baselines match. This patch doesn't fix bug 483558, though.
Assignee | ||
Updated•16 years ago
|
Attachment #367550 -
Flags: review?(bzbarsky)
Comment 15•16 years ago
|
||
Hmm. Why is it ok to ScrollToInitialTarget() in SetValue() even though we haven't reflowed the new text yet?
Assignee | ||
Comment 16•16 years ago
|
||
(In reply to comment #15)
> Why is it ok to ScrollToInitialTarget() in SetValue() even though we
> haven't reflowed the new text yet?
It wouldn't be OK if the new text hasn't been reflowed, but
the reflow is performed due to this FlushPendingNotifications
#0 PresShell::FlushPendingNotifications (this=0xf8e8f0, aType=Flush_Layout)
at /home/karl/moz/dev/layout/base/nsPresShell.cpp:4627
#1 0x00007f7ee9442ae6 in nsEditor::EndUpdateViewBatch (this=0x13d3f10)
at /home/karl/moz/dev/editor/libeditor/base/nsEditor.cpp:4351
#2 0x00007f7ee94494d7 in nsEditor::EndPlaceHolderTransaction (this=0x13d3f10)
at /home/karl/moz/dev/editor/libeditor/base/nsEditor.cpp:956
#3 0x00007f7ee942c7ad in ~nsAutoPlaceHolderBatch (this=0x7fff02ccdf90)
at /home/karl/moz/dev/editor/libeditor/base/nsEditorUtils.h:66
#4 0x00007f7ee9426abb in nsPlaintextEditor::InsertText (this=0x13d3f10,
aStringToInsert=@0x7fff02cce030)
at /home/karl/moz/dev/editor/libeditor/text/nsPlaintextEditor.cpp:792
#5 0x00007f7ee8f375c3 in nsTextControlFrame::SetValue (this=0x13b80c0,
aValue=@0x7fff02cce300)
That only happens when the editor doesn't have eEditorUseAsyncUpdatesMask, but
I see now that bug 151882 seems to want to add that flag.
If we want something that will work with eEditorUseAsyncUpdatesMask, then I
guess the best thing for SetValue() is to set a flag on the nsTextControlFrame
to indicate that a ScrollToInitialTarget() is required and mark the
nsTextControlFrame dirty so that it gets a reflow, and check for the flag in
DoLayout(). I guess it would then make sense to shift the reflow root from the
scroll frame to the nsTextControlFrame?
Assignee | ||
Comment 17•16 years ago
|
||
(In reply to comment #16)
> That only happens when the editor doesn't have eEditorUseAsyncUpdatesMask, but
> I see now that bug 151882 seems to want to add that flag.
I mean bug 174823.
Comment 18•16 years ago
|
||
Yeah, ideally we want to not do a sync flush there.
It's probably ok to leave as-is with a comment explaining why it's ok and another comment in the flag-munging code pointing out that this needs to be addressed if we start using eEditorUseAsyncUpdatesMask. At that point we might want to just use a reflow callback or something.
Assignee | ||
Comment 19•16 years ago
|
||
Added comments suggested in comment 18.
Attachment #367550 -
Attachment is obsolete: true
Attachment #368197 -
Flags: review?(bzbarsky)
Attachment #367550 -
Flags: review?(bzbarsky)
Comment 20•16 years ago
|
||
These screenshots are taken with old nightly before this regression, recent nightly and build with this patch.
My ~/.fonts.conf is same as comment #4 and LANG is ja_JP.UTF-8.
This patch improves 1px, but it's still hard to read as "gooqle."
Updated•16 years ago
|
Flags: blocking1.9.0.9?
Assignee | ||
Comment 21•16 years ago
|
||
(In reply to comment #4)
> Created an attachment (id=311773) [details]
> my ~/.fonts.conf
> This is similar to the style "font-family: "Bitstream Vera Sans",
> "IPAPGothic";"
> but the height of input is different.
Similar, but quite different.
><?xml version="1.0"?>
><!DOCTYPE fontconfig SYSTEM "fonts.dtd">
><fontconfig>
> <alias>
> <family>sans-serif</family>
> <prefer>
> <family>Bitstream Vera Sans</family>
> <family>IPAPGothic</family>
> </prefer>
> </alias>
></fontconfig>
fontconfig gives aliases defined in this way a "weak" "binding", which
means that these fonts are only considered if they support the requested
language, or after aliases that do support the requested language.
% fc-match -s "sans\-serif:lang=en" | head -n 3
Vera.ttf: "Bitstream Vera Sans" "Roman"
ipagp.ttf: "IPAPGothic" "Regular"
DejaVuSans.ttf: "DejaVu Sans" "Book"
% fc-match -s "sans\-serif:lang=ja" | head -n 3
ipagp.ttf: "IPAPGothic" "Regular"
Vera.ttf: "Bitstream Vera Sans" "Roman"
DejaVuSans.ttf: "DejaVu Sans" "Book"
This is intentional so that, for example, Japanese fonts don't get used to
render Chinese text (when fonts for both are installed) even with something
like:
<alias>
<family>sans-serif</family>
<prefer>
<family>Bitstream Vera Sans</family>
<family>IPAPGothic</family>
<family>SimSun</family>
</prefer>
</alias>
The "regression" that makes this bug apparent would most likely be that
Mozilla now interprets the fontconfig setting correctly, whereas previously
Bitstream Vera Sans would have always been the primary font for all
languages and IPAPGothic would have been used for Chinese text.
fontconfig can be persuaded to make Bitstream Vera Sans the primary font for
Japanese text (which would work-around this bug):
<match target="pattern" >
<test name="lang" compare="contains">
<string>ja</string>
</test>
<test name="family" >
<string>sans-serif</string>
</test>
<edit mode="prepend" binding="same" name="family" >
<string>Bitstream Vera Sans</string>
</edit>
<edit mode="prepend" name="family" >
<string>IPAPGothic</string>
</edit>
</match>
% fc-match -s "sans\-serif:lang=ja" | head -n 3
Vera.ttf: "Bitstream Vera Sans" "Roman"
ipagp.ttf: "IPAPGothic" "Regular"
DejaVuSans.ttf: "DejaVu Sans" "Book"
What out that either your gtk/gnome font needs to be "sans-serif", not "sans"
or you need to add another fontconfig rule to test for family "sans", or this
won't work:
fc-match -s "sans:lang=ja" | head -n 3
ipagp.ttf: "IPAPGothic" "Regular"
Vera.ttf: "Bitstream Vera Sans" "Roman"
DejaVuSans.ttf: "DejaVu Sans" "Book"
Assignee | ||
Comment 22•16 years ago
|
||
(In reply to comment #20)
> Created an attachment (id=368380) [details]
> comparison with and without patch
Thanks for this. When I was testing the patch, I foolishly neglected to
remove another change that was causing the code to use different font metrics.
What's happening here is that the text input area is being sized according to
the primary system font for the locale (from GTK and fontconfig), which in
this case is IPAPGothic for Japanese. The metrics of this font as used on
Linux and Mac give the following: (The metrics in the hhea table and the typo
metrics in the OS/2 table are the same in this font.)
ascent/ascender: 0.88 em
descent/descender: -0.12 em
line height: 1.00 em
This is a small descent when compared with typical Latin fonts. I assume this
descent was chosen to be representative of the Japanese glyphs in the font and
it would be a reasonable value I think for a font that had no Latin glyphs.
(This font does have Latin glyphs actually and the descent is not low enough
to fit the descenders of Latin glyphs within this font. I don't know whether
that really makes it "wrong" though, as the font is designed primarily for
Japanese text.)
Further there is no vertical spacing between the em heights for each line.
Is this a normal feature of Japanese fonts?
(It is not common for Latin fonts.)
(On Windows, different metrics are used, and I had been incorrectly testing
with those metrics: The winAscent is the same as the other ascent metrics, but
the winDescent is 0.196em, which was giving a line height of 1.076em, which
happened to provide enough space to see another pixel. Changing the Linux/Mac
code to use these Windows metrics is not the solution here because these
metrics have their own sets of problems, and other fonts will still have the
same problem as seen here.)
The input area is one line-height high. Normally most Latin letters fit
between the ascent and descent, and the line height sometimes provides a
little padding beyond that for unusual glyphs.
When Latin words are entered into the text input, the system's (fontconfig's)
font for English text is used for those words. That font has a similar em
height (modulo hinting and non-scalable bitmap fonts), but other metrics
differ. In this case, that font is Bitstream Vera Sans, and the metrics used
from that font are:
ascender: 0.928 em (from hhea table)
descender: -0.236 em (from hhea table)
line height: 1.200 em (from typo metrics in OS/2 table)
Without attachment 368197 [details] [diff] [review], the baseline of the Latin text was moved down
~0.048em to accommodate the additional ascent here. It seems that, after
rounding, this amounted to a 1 pixel shift. This shift increased the number
of pixels truncated off the bottom of the text input area.
With attachment 368197 [details] [diff] [review], the baseline of the Latin text is held consistent with
the baseline intended for Japanese text. That means that fewer pixels are
truncated at the bottom, but there simply isn't enough space allocated below
the baseline in the text input to draw these pixels.
Assignee | ||
Comment 23•16 years ago
|
||
The reason why attachment 368197 [details] [diff] [review] is not enough to make Latin text completely
visible here is that baseline alignment is not enough to make Latin text
visible in a text input with a baseline intended for Japanese text.
Possible improvements to this situation are:
a) Position the text so that the em height of the first character is visible.
A benefit of this approach is that, because the em height is similar for
fonts for all languages, this is something that is achievable even in a
text input sized for a different language.
Another benefit is that this could be consistent with a ScrollIntoView
implementation that makes the em height visible (bug 483558).
An issue with this approach is that baselines of text within text-input
boxes would sometimes not align with baselines of adjacent text.
Another issue with this approach is that the em height only applies to a
particular font, so the em ascent and descent could be different for other
characters, but those other characters are aligned by baseline and so may
not remain visible. (A similar ScrollIntoView implementation would make
them visible when the cursor was moved to the obscured characters though.)
b) It seems that not all of the space available in the location bar is being
made available to the text input. Making the text input occupy the entire
available space would seem sensible.
I guess we'd want some way to center the text if the text input ends up
higher than one line-height. With the current implementation that could be
done by applying padding to the child div, but that feels a little messy.
c) The height and baseline of the text input could be determined by
considering fonts for both the current locale and Latin text.
I wonder whether we'd want to do this for all text inputs or only those
where we'd expect Latin characters.
Ideally, like (b), this would need to have some means of shifting the text
down to a baseline lower than the baseline of the child div.
Comment 24•16 years ago
|
||
(In reply to comment #22)
> Further there is no vertical spacing between the em heights for each line.
> Is this a normal feature of Japanese fonts?
> (It is not common for Latin fonts.)
Yes. Maybe, Internal Leading/External Leading of most major Japanese fonts are zero. Therefore, line-height: auto; was too tight layout before Mozilla 1.0. For the issue, we have special code for such fonts:
http://hg.mozilla.org/mozilla-central/file/956071116564/layout/generic/nsHTMLReflowState.cpp#l2024
Assignee | ||
Comment 25•16 years ago
|
||
(In reply to comment #24)
> For the issue, we have special code for such fonts:
> http://hg.mozilla.org/mozilla-central/file/956071116564/layout/generic/nsHTMLReflowState.cpp#l2024
Thanks, Masayuki. It seems that hinting and rounding is causing us to fail the "if (!internalLeading && !externalLeading)" test. If we pass this test we'll have 20% more space. I'll see if I can find anything fundamentally wrong with the rounding.
(gdb) p mMetrics
$308 = {xHeight = 7.0000000000000009, superscriptOffset = 7.0000000000000009,
subscriptOffset = 2, strikeoutSize = 1, strikeoutOffset = 6,
underlineSize = 1, underlineOffset = -1, height = 0, internalLeading = 1,
externalLeading = 0, emHeight = 13, emAscent = 11.44,
emDescent = 1.5600000000000005, maxHeight = 14, maxAscent = 12,
maxDescent = 2, maxAdvance = 13, aveCharWidth = 8, spaceWidth = 4,
zeroOrAveCharWidth = 8}
(gdb) p this->mStyle
$309 = {style = 0 '\0', systemFont = 1 '\001', printerFont = 0 '\0',
familyNameQuirks = 0 '\0', weight = 400, stretch = 0,
size = 13.333333333333334, langGroup = {<nsACString_internal> = {
mData = 0x2540e08 "x-unicode", mLength = 9,
mFlags = 5}, <No data fields>}, sizeAdjust = 0}
Assignee | ||
Updated•16 years ago
|
Attachment #368197 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 26•16 years ago
|
||
Comment on attachment 368197 [details] [diff] [review]
patch with more comments
Removing review request on this. There is a discontinuity in the function for scroll position in ScrollToInitialTarget() wrt the height of the control when textareas have more than one line of text.
Comment 27•16 years ago
|
||
(In reply to comment #25)
> If we pass this test we'll
> have 20% more space. I'll see if I can find anything fundamentally wrong with
> the rounding.
On Windows, we sometimes don't have "20% more space."
Is this related?
Or should I file another bug?
Updated•16 years ago
|
Attachment #368855 -
Attachment mime type: text/html → text/html; charset=Shift_JIS
Comment 28•16 years ago
|
||
We don't have "20% more space" with MS PGothic 32px.
Assignee | ||
Updated•16 years ago
|
Assignee | ||
Comment 29•16 years ago
|
||
Comment on attachment 368197 [details] [diff] [review]
patch with more comments
An updated version of this patch is in bug 349259. (At least it completely fixes that bug, but I think there may be a better solution.)
Attachment #368197 -
Attachment is obsolete: true
Assignee | ||
Comment 30•16 years ago
|
||
(In reply to comment #27)
> Created an attachment (id=368855) [details]
> testcase for 20% more space
> On Windows, we sometimes don't have "20% more space."
> Is this related?
> Or should I file another bug?
Yes, this is the same 20% being only added sometimes due to rounding.
There's more than one issue in this bug so it would be worth having another bug for that issue.
Comment 31•16 years ago
|
||
(In reply to comment #30)
> There's more than one issue in this bug so it would be worth having another bug
> for that issue.
Filed Bug 485335.
Updated•16 years ago
|
Flags: blocking1.9.0.10? → blocking1.9.0.10+
Updated•16 years ago
|
Flags: blocking1.9.0.10+
Assignee | ||
Updated•15 years ago
|
Reporter | ||
Comment 32•15 years ago
|
||
The issue at URL bar and status bar now seems to be fixed between ddfecbc93934 and 7c24dc44ca00.
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ddfecbc93934&tochange=7c24dc44ca00
However attachment 312030 [details], attachment 311770 [details], and attachment 376361 [details] (bug 485335) still have this issue with IPAPMincho and IPAPGothic fonts which are proportional sans-serif and serif fonts respectively.
Assignee | ||
Comment 33•15 years ago
|
||
(In reply to comment #32)
> The issue at URL bar and status bar now seems to be fixed between ddfecbc93934
> and 7c24dc44ca00.
Bug 524107 perhaps.
I don't know whether that would be expected/intentional though.
Reporter | ||
Comment 34•13 years ago
|
||
Attachment 312030 [details], attachment 311770 [details] seems to be shown properly.
However I don't know which bug fix this issue.
Reporter | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•