212302 - some links don't have underline decoration

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Joseph Izso]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624

Links should have underline decoration in hover state. In Mozilla some links
don't have underline, some still have underline after onmouseout.

Reproducible: Couldn't Reproduce

Steps to Reproduce:
1.
2.
3.



Expected Results:  
Links must have text-decoration: underline in hover state

Comment 1

[Joseph Izso]

Attachment: occured page #1

Comment 2

[Joseph Izso]

Attachment: occured page #2

Comment 3

[Adam Becevello]

WFM with attached pages: 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030710

I notice that you are importing a stylesheet (style.css) into your page.  Is
there anything in there that has 'text-decoration:none' that relates to links? 
I can get the links to have no underlining on hover by putting 
A:hover {text-decoration:none;} into the referenced stylesheet.

Comment 4

[Bill Mason]

Attachment: Minimized testcase

Don't understand the issue, but here's the minimized code.

Oddly, if I remove the body { font-size : 8pt; } declaration then it WFM
20030627 PC/WinXP.

Comment 5

[Bill Mason]

Confirming new since I see the same problem at the given URL, 20030627 PC/WinXP.
Added kw testcase.

Comment 6

[Adam Becevello]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030710

I've narrowed down the problem.  It seems that every font size in the body works
except for 8pt.  As well, the <code class="attribute"> tags do not affect this bug.

If some other text-effect (such as font-style:italic;) is added to the 'a:hover,
a:focus' CSS rule, this bug goes away.  

I believe the main problem we're seeing is with the <pre> tags.  If these are
removed this bug goes away.  It seems 8pt. fonts that have
text-decoration:underline while hovered within <pre> tags causes problems.

Comment 7

[Adam Becevello]

Attachment: further reduced testcase

Further minimized code.
If the <pre> tags are removed, this bug goes away.
This fact that these are links is not important.  If this is just normal text
with text-decoration:underline; on its hover CSS rule, this bug still exists.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Sounds like the next line is painting over the decorations or something?  I
can't reproduce this with a current Linux build, so it may be Windows-only....

Comment 9

[Alex Hirzel]

*** This bug has been marked as a duplicate of 196249 ***

Comment 10

[Simon Montagu :smontagu]

Comparing the minimized testcases, attachment 127486 [details] and attachment 130649 [details], this
doesn't seem to be a dupe.

Comment 11

[Arthur Wiebe]

I've had a problem with the underline not working when the link contains spaces.
If the link does not contain spaces then the CSS underline hover works.

Comment 12

[Mark Annand]

Firefox: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) 
Gecko/20040602 has a problem with the following site:

http://www.bathspa.ac.uk/

The site contains lists of underlined links, the links having a css hover 
effect that changes their colour. Hovering over a link and then leaving it 
seems to cause a repaint of the link's colour that catches the underline of the 
link above it, erasing it.

Steps to reproduce

1 Open that site
2 Run the mouse cursor over a list of links from top to bottom, underlining 
disappears in a ragged way.
3 Do the opposite and you'll make it reappear

For good measure this bug depends on text size settings - change those in 
the 'view' menu and the behaviour vanishes. It just happens to show it, on this 
machine, at the default text size settings for the page

If this is the right bug for this problem it's probably quite visible on a 
number of sites and therefore more than trivial

Comment 13

[Mark Annand]

Attachment: Screenshot of page as it should be

Links all underlined

Comment 14

[Mark Annand]

Attachment: Screenshot of missing link underlining - Comment 12

The underlining goes missing on any part of a link above one that has been
moused over

Comment 15

[Mark Annand]

I've reproduced the behaviour from comment 12 on another Windows 2000 machine 
with a CRT monitor, using Firefox /20040601, by altering the refresh rate and 
screen resolution, and resizing the text size in Firefox.

The 'Comment 12' machine has a flat screen running at 1280x1024 and 60Hz 
refresh rate - possibly this only shows up at a particular resolution?

Comment 16

[Hideo Saito]

Attachment: patch

This patch trims and adjusts the underline offset, if underline separates too
far from the bottom of the font, and the baseline of the font does not shift
upward.

Comment 17

[Hideo Saito]

I think that the patch of Bug 156943 has a regression, since the baseline of the
several CJK fonts shift upward and the underline of that font goes out of the
input box.

Comment 18

[Hideo Saito]

Attachment: testcase for patch

Comment 19

[Hideo Saito]

Attachment: screenshot for patch

Comment 20

[Hideo Saito]

Comment on attachment 159983 [details] [diff] [review]
patch

I want to do following two things.

1. to calculate the text metrics of decent, font height, internal leading and
underline offset instead of shifting the base line.
2. to move underline to the appropriate position if underline separates too far
from the bottom of the font.

Comment 21

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

This patch fixes bug 312838 too.

Rbs: Please review it.

Comment 22

[rbs]

Comment on attachment 159983 [details] [diff] [review]
patch

This patch is not written up as I like. It shouldn't have that "#ifdef" and should be written within the setting/spirit
of gDoingLineheightFixup.

Comment 23

[Hideo Saito]

Attachment: patch

Comment 24

[rbs]

Comment on attachment 207736 [details] [diff] [review]
patch

r=rbs.  I applied the pacth and it fixed the testcases here. Some nits:

+      // if underline positioned too near the text, descent position
                                 ^
                              ...is...


+      // only do this for CJK to minimize possible risk

Remove this since it is not true anymore. There is not IsCJK test anymore.

       mUnderlineOffset = descentPos;
+      nscoord extraOff = mUnderlineSize - mUnderlineOffset - mMaxDescent;

Simply call this 'extra' or 'extraOffset' in full. 
(extraOff gives the impression that there is extraOn.)


+      // if underline positioned too far the text, descent position
                                 ^
                              ...is...

// if underline positioned IS too far the text, descent position

============
PS: As the comment in that code says, gDoingLineheightFixup was meant to "be eliminated in future when we choose to stay with one normal lineheight calculation method." Hope that happens some day...

Comment 25

[rbs]

Nit on my nit: use "IS too far/near FROM". (These are trivialities of course.)

Comment 26

[Hideo Saito]

Attachment: fix for check in

Comment 27

[rbs]

Comment on attachment 207782 [details] [diff] [review]
fix for check in

r+sr=rbs

Comment 28

[Hideo Saito]

Thanks rbs, could you check in to the trunk?

Comment 29

[rbs]

Checked in the trunk.

Comment 30

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

Let's go to 1.8.1 if we will not have regression reports.

Comment 31

[Jesse Ruderman]

*** Bug 241627 has been marked as a duplicate of this bug. ***

Comment 32

[[e-mail]]

Does this fix bug 35735?

Comment 33

[Hideo Saito]

(In reply to comment #32)
> Does this fix bug 35735?

Yes, the problem which an underline disappears or is not displayed was fixed on Windows.

Comment 34

[Jesse Ruderman]

*** Bug 35735 has been marked as a duplicate of this bug. ***

Comment 35

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

-> v.

Comment 36

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

Comment on attachment 207782 [details] [diff] [review]
fix for check in

We should fix this bug on 1.8 branch too. Because bug 312838 is accessibility bug for IME users who are using non-CJK locale Windows.

Comment 37

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

checked-in to 1.8 branch.

Comment 38

[Martijn Wargers (dead)]

This bug basically started reappearing again as soon as cairo was turned on for windows builds. I've filed bug 389690 for it.