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

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: scy.hemi, Assigned: masayuki)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [to be fixed by 417014], URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

11 years ago
Created attachment 285227 [details]
Test HTML

testcase
(Reporter)

Comment 2

11 years ago
Created attachment 285228 [details]
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
(Reporter)

Updated

11 years ago
Version: unspecified → Trunk
(Reporter)

Updated

11 years ago
Severity: major → normal

Comment 3

11 years ago
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
(Reporter)

Updated

11 years ago
Component: GFX: Win32 → GFX: Thebes
QA Contact: win32 → thebes

Comment 4

11 years ago
Created 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 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

11 years ago
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.
Attachment #286152 - Attachment description: Testcase screenshot of MingLiU and MS JhengHei fonts on Gran Paradiso Alpha 8 and Fx 2.0.0.8 pn Windows XP Home → Testcase screenshot of MingLiU and MS JhengHei fonts on Gran Paradiso Alpha 8 and Fx 2.0.0.8 on Windows XP Home
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.
Assignee: nobody → masayuki
Status: UNCONFIRMED → NEW
Ever confirmed: true
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   }
Blocks: 396809
No longer blocks: 396809
Status: NEW → ASSIGNED
Depends on: 402524
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
Flags: blocking1.9?
see bug 402524. it should fix this bug.

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Created attachment 298630 [details]
underline and line-through of text using mac fonts
Created attachment 298636 [details]
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.
Attachment #298630 - Attachment is obsolete: true
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.
(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.
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 :-(
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.
(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).
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.
Depends on: 417014
I'll fix this bug in bug 417014, which need the blacklist of bad fonts.
If you are interesting, please check the bug.

Updated

11 years ago
Flags: tracking1.9+ → blocking1.9-
Whiteboard: [to be fixed by 417014]
now, this bug should be fixed by bug 417014.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 20

11 years ago
It's fixed! Thank you Masayuki!!
You need to log in before you can comment on or make changes to this bug.