400141 - Link underline and text-decoration: underline are too high for CJK characters

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Hemiola SUN]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre

Since 3.0a?, I found that the underline of links and "text-decoration: underline" is too high for CJK characters and will make the characters sometimes unidentifiable.

Please see attached test html and the screenshoot.

Reproducible: Always

Steps to Reproduce:
1. Always
2.
3.
Actual Results:  
The underline is too high.

Expected Results:  
The underline should be positioned lower.

Comment 1

[Hemiola SUN]

Attachment: Test HTML

testcase

Comment 2

[Hemiola SUN]

Attachment: testcase screenshot

tested on Firefox 3.0a9pre and 2.0.0.7, in Windows XP SP2, with PMingLiu font, default font in Traditional Chinese XP system

Comment 3

[Wenbin Leo]

As this bug happens on Firefox3.0a9pre, which uses Cairo APIs. Component should be GFX: Thebes, not GFX: Win32.
Like https://bugzilla.mozilla.org/show_bug.cgi?id=380026#c2

Comment 4

[coolcd05a]

Attachment: Testcase screenshot of MingLiU and MS JhengHei fonts on Gran Paradiso Alpha 8 and Fx 2.0.0.8 on Windows XP Home

Four screenshots were taken and put together for comparison. On the left part of the attached image, MingLiU was used for Chinese, arial for English in Gran Paradiso Alpha 8 and Firefox 2.0.0.8. On the right part, MS JhengHei was used. All tests were done on traditional Chinese version of Windows XP Home.

It's clear that the bottom of MingLiU Chinese font overlapped too much with the underline in Gran Paradiso Alpha 8. The following two screenshots show the situation of using MS JhengHei font. It looks slightly better.

I think it's clear that font and underline rendering still needs to be improved in Gran Paradiso Alpha 8.

Comment 5

[coolcd05a]

Comment on attachment 286152 [details]
Testcase screenshot of MingLiU and MS JhengHei fonts on Gran Paradiso Alpha 8 and Fx 2.0.0.8 on Windows XP Home

Four screenshots were taken and put together for comparison. On the left part
of the attached image, MingLiU was used for Chinese, arial for English in Gran
Paradiso Alpha 8 and Firefox 2.0.0.8. On the right part, MS JhengHei was used.
All tests were done on traditional Chinese version of Windows XP Home.

It's clear that the bottom of MingLiU Chinese font overlapped too much with the
underline in Gran Paradiso Alpha 8. The right part of the image shows the
situation of using MS JhengHei font. It looks slightly better.

I think it's obvious that font and underline rendering still needs to be improved
in Gran Paradiso Alpha 8.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that this is not really our bug. The underline position is font specified position. However, I found such code in old gfx:

http://lxr.mozilla.org/mozilla1.8/source/gfx/src/windows/nsFontMetricsWin.cpp#3774
> 3774     if (gDoingLineheightFixup) {
> 3775       if(IsCJKLangGroupAtom(mLangGroup.get())) {
> 3776         mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition, 
> 3777                                                  oMetrics.otmDescent + oMetrics.otmsUnderscoreSize) 
> 3778                                                  * dev2app);
> 3779         // keep descent position, use it for mUnderlineOffset if leading allows
> 3780         descentPos = NSToCoordRound(oMetrics.otmDescent * dev2app);
> 3781       } else {
> 3782         mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition*dev2app, 
> 3783                                                  oMetrics.otmDescent*dev2app + mUnderlineSize));
> 3784       }
> 3785     }

I'll check this after my current work.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And here:

http://lxr.mozilla.org/mozilla1.8/source/gfx/src/windows/nsFontMetricsWin.cpp#3830
> 3830   if (gDoingLineheightFixup) {
> 3831     if (mInternalLeading + mExternalLeading > mUnderlineSize &&
> 3832         descentPos < mUnderlineOffset) {
> 3833       // If underline positioned is too near from the text, descent position
> 3834       // is preferred, but we need to make sure there is enough space
> 3835       // available so that underline will stay within boundary.
> 3836       mUnderlineOffset = descentPos;
> 3837       nscoord extra = mUnderlineSize - mUnderlineOffset - mMaxDescent;
> 3838       if (extra > 0) {
> 3839         mEmDescent += extra;  
> 3840         mEmHeight += extra;
> 3841         mMaxDescent += extra;
> 3842         mMaxHeight += extra;
> 3843       }
> 3844     } else if (mUnderlineSize - mUnderlineOffset > mMaxDescent) {
> 3845       // If underline positioned is too far from the text, descent position
> 3846       // is preferred so that underline will stay within boundary.
> 3847       mUnderlineOffset = mUnderlineSize - mMaxDescent;
> 3848     }
> 3849   }

Comment 8

[Po-chiang Chao [:bobchao]]

This bug (In reply to comment #6)
> I think that this is not really our bug. The underline position is font
> specified position. 

We should pay more attention on this bug. MingLiU is the default font in XP, and MS JhengHei is the default in Vista. this will leading the users think it's Firefox's fault.

request blocking 1.9

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

see bug 402524. it should fix this bug.

Comment 10

[John Daggett (:jtd)]

Attachment: underline and line-through of text using mac fonts

Comment 11

[John Daggett (:jtd)]

Attachment: underline and line-through including original testcase

On the Mac there are a couple of rendering artifacts with the underline but I don't see the same problems that show up on Windows.

Comment 12

[John Daggett (:jtd)]

On Windows the "underline and line-through" testcase renders with a disjoint underlining, at large sizes the underline below the Chinese is *lower* than the underline below the Latin text to the left of it.  FF2 has the same problem.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> On Windows the "underline and line-through" testcase renders with a disjoint
> underlining, at large sizes the underline below the Chinese is *lower* than the
> underline below the Latin text to the left of it.  FF2 has the same problem.

I think that you misunderstand. The issue is only on Quirks mode. In standards mode, the issue should be gone.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

mmm.. current my patch breaks Meiryo's underline position, but Fx2 doesn't break it. I failed to port the underline position logic from legacy win gfx :-(

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

er, even if we should fix the disjoint issue on Quriks mode too, it is out of scope of this bug. Because it is layout, not gfx.

Comment 16

[John Daggett (:jtd)]

(In reply to comment #13)

> I think that you misunderstand. The issue is only on Quirks mode. In standards
> mode, the issue should be gone.

I'm confused, the testcase here should render in full standards mode and my testcase should render in quirks mode (no doctype).

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

In quriks mode, the decoration lines are rendered every elements. Otherwise, the decoration lines are rendered only for the elements which are specified "text-decoration" property. The quriks mode behavior is invalid for CSS spec, but it is really needed for backward compatibility. However, the elements which are not specified "text-decoration" property should use ancestor ("text-decoration" specified) element's text metrics, maybe.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'll fix this bug in bug 417014, which need the blacklist of bad fonts.
If you are interesting, please check the bug.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

now, this bug should be fixed by bug 417014.

Comment 20

[Hemiola SUN]

It's fixed! Thank you Masayuki!!