1305464 - Indentation looks wrong in rendered diff when using 'Droid Sans Mono'

Status: RESOLVED INVALID

(MozReview Graveyard :: Review Board: User Interface, defect)

Description

[Botond Ballo [:botond]]

Attachment: Screenshot showing wrong-looking indentation

The attached screenshot is of a MozReview diff view [1].

The diff contains a hunk that adds a new method, NotifyPinchGesture. The method declaration takes up several lines, and subsequent lines are indented so that the types of the parameters line up.

However, the way the diff is rendered, it looks like the subsequent lines are actually indented by one fewer columns than what you'd want.

This wrong-looking indentation can only be seen in MozReview; if you look at the raw diff, you can see the lines are indented correctly.

A closer look at the MozReview diff rendering reveals that, even though the font used to render the code in the diff looks like a fixed-width font (i.e. the shapes of the glyphs are what you'd expect from a fixed-width font, and not from a variable-width font), the font isn't actually fixed-width: characters don't actually line up in columns the way they should. This is likely the underlying cause of the wrong-looking indentation.

[1] https://reviewboard.mozilla.org/r/80846/diff/1/

Comment 1

[:glob ✱]

i don't see this issue - the text is correctly aligned for me.

> the font isn't actually fixed-width: characters don't actually line up in columns the way they should

the font used is "monospace" - ie. whatever you have configured as the fixed-width font in your browser.
spaces are used for alignment; the only markup within the <pre> element are <span>s.

what font is being used to render the code?  (use the 'fonts' tab in devtools when inspecting the <pre> element)

are you able to experiment with changing to a different fixed-width font (about:preferences -> content -> advanced)
i'm using 'source code pro' at 13pt.


i suspect the bold width of the font you're using is slightly wider than normal weighted characters, resulting in the misalignment.  you can see this in your screenshot on line 189 .. look at where the "l" in "bool" aligns with the characters in the next line.


if you're using the default monospace font, we should consider explicitly setting the font to one we know doesn't show this behaviour.

Comment 2

[Botond Ballo [:botond]]

(In reply to Byron Jones ‹:glob› from comment #1)
> what font is being used to render the code?  (use the 'fonts' tab in
> devtools when inspecting the <pre> element)

It's Droid Sans Mono. This matches the setting for "monospace" in about:preferences -> content -> advanced.

On a different system where "monospace" is set to "Courier New", the characters are correctly aligned.

I'm not sure what the default is, and on which system I changed it (or it could have been both).

I tried some other monospace fonts like DejaVu Sans Mono and Liberation Mono, and it's fine with those as well. So perhaps we just need to blacklist Droid Sans Mono?

Comment 3

[Botond Ballo [:botond]]

It looks like the problem is that Droid Sans Mono doesn't have a dedicated bold version, but rather the bold characters are produced algorithmically from the regular characters, and this process changes the width. See e.g. http://askubuntu.com/questions/100672/how-to-prevent-automatic-bold-version-of-a-font-to-be-wider-than-regular-havin.

Comment 4

[:glob ✱]

"droid sans mono bold" does exist - https://www.onlinewebfonts.com/download/5e4830520347d02cd492a9c15fdc967b (note the license: "This font software is licensed to you by Ascender Corporation for your personal or business use on up to five personal computers").

incorrect widths of bold characters in firefox was fixed in bug 1062108, which is an android only fix as far as i can tell.

http://searchfox.org/mozilla-central/source/modules/libpref/init/all.js indicates that "droid sans mono" is only used as the default font on android, elsewhere it's "courier new".

unfortunately we cannot blacklist fonts with css, and given "droid sans mono" isn't the default monospace font i don't think we can do much more here i'm sorry.