136935 - :hover paints over previous link's underline

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[James Lariviere]

A link has its underline painted over by links on following lines. (XP build 4-11)

This is happens with smaller fonts sizes.

I'll post a screenshot.

I think its related to the fix for bug 5693.

Comment 1

[James Lariviere]

Attachment: espn screenshot -- XP build 04-11-03

A site I work on also suffers from this problem.

www.mutualfundsnet.com at the footers 2nd line.

Steps:
1) hover over a link
2) after you hover off link the link lines above it will now be missing.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is almost certainly unrelated to bug 5693, and is almost certainly related
to the line-height being set too small on those lines...

Comment 3

[James Lariviere]

hey boris...

What changed over the past day that would have caused this (I noticed that
someone else has mentioned this in bug 5693)?  Nothing catches my untrained eye :-).

Could this have been a problem before and david's patch exposed the line height
problem?

I'll download yesterdays build just to verify that it wasn't already there.

Comment 4

[James Lariviere]

Yep checked the win 4-9 and 4-10 builds and there are no problems rendering
:hover over the small links.

Again I think that david's patch exposed the problem because you do not see the
messed rendering until after you hover off the link.

Line height is the problem because with other links with larger values to don't
render over the previous link's underline.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We're probably sending fewer :hover state changed notifications now when moving
from parent to child (although I don't know why we'd have repainted the parent
in the past), and this has exposed the problem that already exists related to
any text frames that overflow their bounds (bug 109184).

The problem goes away if you trigger a repaint (e.g., by moving another window
over the Mozilla window and then moving it back), right?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

From bug 5693:

------- Additional Comment #190 From Warner Young 2002-04-11 11:04 -------

dbaron, on the site http://www.aberdeeninc.com, when I move the mouse over the
various links, sometimes I get links that don't underline, and other times, the
underlines don't go away after I move the mouse away.  I don't know if this is
related or not.  Trunk build, 2002041103, Win98.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, I just noticed that shanjian landed bug 76097 yesterday, which is much
more likely to be the cause of this regression than bug 5693 (since I really
don't see why we would have repainted the parent in the past).

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

FWIW, what would cause this is a reduction in whichever font metrics determine
the size of the text frame.  I suspect something in bug 76097 caused the metrics
that determine the size of the frame to be smaller, causing the underline (and
perhaps also some of the upper extents) of the text to be outside the text frame.

A similar problem happens in much rarer cases on Linux - see bug 109184.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, now that I'm looking at the patch, it might be that the change that
caused this was actually:

@@ -3262,7 +3271,10 @@
     mStrikeoutSize = PR_MAX(onePixel, NSToCoordRound(oMetrics.otmsStrikeoutSize
* dev2app));
     mStrikeoutOffset = NSToCoordRound(oMetrics.otmsStrikeoutPosition * dev2app);
     mUnderlineSize = PR_MAX(onePixel,
NSToCoordRound(oMetrics.otmsUnderscoreSize * dev2app));
-    mUnderlineOffset = NSToCoordRound(oMetrics.otmsUnderscorePosition * dev2app);
+    if (gDoingLineheightFixup)
+      mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition,
oMetrics.otmDescent) * dev2app);
+    else
+      mUnderlineOffset = NSToCoordRound(oMetrics.otmsUnderscorePosition * dev2app);
 
     // Begin -- section of code to get the real x-height with GetGlyphOutline()
     GLYPHMETRICS gm;

Comment 10

[Shanjian Li]

Attachment: patch

Comment 11

[Shanjian Li]

It is most likely that my way of calculating underline position exceed current
line area. I propose a patch which might fix the problem. Because I could not
reproduce the problem on my machine, david, could you take care of it?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

r=dbaron, but I have no way to test since I don't even have a windows machine.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Back to shanjian, since I have no way of testing this either.  (One thing to
check, though:  mUnderlineOffset is a positive number, not a negative one, right?)

Comment 14

[Shanjian Li]

Attachment: update patch

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, that agrees better with bug 11168 comment 25.  r=dbaron

Comment 16

[Shanjian Li]

chris/marc, could you give sr=?

Comment 17

[Marc Attinasi]

Comment on attachment 78835 [details] [diff] [review]
update patch

sr=attinasi

Comment 18

[Shanjian Li]

fix checked into trunk. 

Comment 19

[José Jeria]

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

Comment 20

[Kevin Gadd]

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