1322740 - At some full-zoom levels, line numbers are not aligned with lines

Status: RESOLVED FIXED

(Webtools :: Searchfox, defect)

Description

[Botond Ballo [:botond]]

Attachment: searchfox-line-numbers.png

STR:
  1. View a file in searchfox such as 
     http://searchfox.org/mozilla-central/source/layout/base/nsIPresShell.h
  2. Zoom in to a 120% full-zoom
  3. Select a line

Expected results:
  The line number is aligned with the line it pertains to

Actual results:
  The line number is misaligned by a significant amount

Please see the attached screenshot to illustrate.

The alignment seems to be fine at zoom levels of 100% or 110%.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Oddly, I don't see this. What platform are you on? Does it reproduce in a fresh profile?

Comment 2

[Botond Ballo [:botond]]

Hmm. It indeed does not reproduce in a fresh profile. However, in my default profile it reproduces even in safe mode, so I can't point the finger at an add-on. Weird!

I'm on Debian stable with KDE.

Comment 3

[Botond Ballo [:botond]]

Attachment: searchfox-line-numbers2.png

Here's what it looks like near the top of the page: observe that the entire line number column is shifted down by quite a bit.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Could you try clearing your cache? Also, do you have any special user style sheets?

Comment 5

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)
> Hmm. It indeed does not reproduce in a fresh profile. However, in my default
> profile it reproduces even in safe mode, so I can't point the finger at an
> add-on. Weird!

The issue also happens on my default profile on another machine, so it's not a one-off this-profile-is-messed-up thing.

Comment 6

[Botond Ballo [:botond]]

(In reply to Bill McCloskey (:billm) from comment #4)
> Could you try clearing your cache? 

That did not help.

> Also, do you have any special user style sheets?

I do have a user style sheet in Stylish, but as mentioned, the issue is still there when running with addons disabled (which presumably disables any Stylish style sheets).

Comment 7

[Botond Ballo [:botond]]

Note that the issue occurs with both Firefox Nightly and Firefox Release.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

I looked into this on Botond's machine. It looks like what's happening is that the code part sometimes has tokens rendered with font-weight:600 (for example the AsyncScrollbarDragRejected function name near the bottom of [1]). In these cases, the font is rendered at a slightly taller height than the 15.6px it's supposed to be rendered at (IIRC it was rendered at 16.01px or so). This causes the enclosing <code> elements to also get a little taller, and when combined across the entire page, the content in the <td class="code"> has more vertical height than the content in <td id="line-numbers"> (which has no bold elements). This is the root of the problem.

In the searchfox-line-numbers2.png screenshot there is a gap above the top of the line numbers, and that's a result of the contents getting the implicit "vertical-align: middle" in their respective td's. There is a corresponding gap at the bottom, and putting a vertical-align:top just moves the top gap to the bottom.

Anyway we can fix this on site by forcing a max-height:15.6px on the <code> elements but that seems like a ugly workaround for a Firefox font rendering bug. I think the proper fix is to make sure Firefox doesn't render these heavier fonts so much taller, or at least doesn't do layout with that height.

[1] https://searchfox.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp

Comment 10

[Botond Ballo [:botond]]

Thanks Kats for investigatin!

cc :jfkthame who might be able to comment on the font sizing issue.

Comment 11

[Jonathan Kew [:jfkthame]]

It sounds like the page is assuming changes in font-weight will not affect computed line-height; this is not a safe assumption, because in general, changing font-weight may result in using a different font face, and there's no guarantee that two different font faces (even if they're in the same font-family) will have the same metrics (font ascent/descent/line-gap) in their tables.

Botond, what actual font is used here on your machines? The CSS has font-family:monospace, but what that resolves to in the end will depend on the fontconfig setup. I suspect that if we answer this, and then look at the regular and bold faces of the font family that leads to, we'll find a discrepancy in metrics.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Interesting, I didn't know that. I guess this does need to be fixed in Searchfox after all.

Comment 13

[Botond Ballo [:botond]]

(In reply to Jonathan Kew (:jfkthame) from comment #11)
> Botond, what actual font is used here on your machines? The CSS has
> font-family:monospace, but what that resolves to in the end will depend on
> the fontconfig setup. I suspect that if we answer this, and then look at the
> regular and bold faces of the font family that leads to, we'll find a
> discrepancy in metrics.

I have set my monospace font to Courier New in the Firefox preferences.

I guess that explains these observations:

(In reply to Botond Ballo [:botond] from comment #5)
> (In reply to Botond Ballo [:botond] from comment #2)
> > Hmm. It indeed does not reproduce in a fresh profile. However, in my default
> > profile it reproduces even in safe mode, so I can't point the finger at an
> > add-on. Weird!
> 
> The issue also happens on my default profile on another machine, so it's not
> a one-off this-profile-is-messed-up thing.

In the default profiles on both of my machines, I have set the monospace font to Courier New, but in a fresh profile I left it at its default, which is something else.

Comment 14

[Jonathan Kew [:jfkthame]]

Checking the OS/2 table in Courier New (the version I have on macOS, at least), I find:

Courier New:
    <sTypoAscender value="1255"/>
    <sTypoDescender value="-386"/>

Courier New Bold:
    <sTypoAscender value="1297"/>
    <sTypoDescender value="-428"/>

So the typographic metrics provided by the bold face give a significantly larger overall vertical height (0.84228515625em vs. 0.80126953125em, as these are font units based on a 2048-unit em-square), so it's not entirely surprising that it can result in a larger line-height (depending exactly how pixel-rounding works out).

Comment 15

[Kartikaya Gupta (email:[e-mail])]

The fix to searchfox should be deployed now.

Comment 16

[Botond Ballo [:botond]]

Confirmed fixed on my end. Thanks!

Comment 17

[Boris Zbarsky [:bzbarsky]]

This broke line alignment for anyone with a minimum font size set to larger than 12px.  See bug 1434816.