1175991 - cset IDs on treeherder are inconsistently ellipsized (some are, some aren't), when browser chooses a wider font than we expect (e.g. due to bug 1173826)

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P4)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Visit this URL:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3562a09b2bf3
or just visit the main m-i treeherder page and scroll down.

ACTUAL RESULTS: csets IDs are inconsistently ellipsized. Screenshot coming up. It looks like they all *nearly* fit, but in most cases they're barely too wide and trigger an ellipsis.

EXPECTED RESULTS:
No ellipses. Or: consistent ellipses.


(Maybe these should be rendered in a monospace font, so they're all the same width? At least then, if they're ellipsized, they'll all be ellipsized in the same way.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot

Here's a screenshot. Note that the last cset has no ellipsis (and _barely_ fits). The others have an ellipsis starting an an inconsistent x-position, which looks awkward. (They may have different numbers of characters being ellipsized, too; not sure.)

Comment 2

[Daniel Holbert [:dholbert]]

I think this is a recent regression; adding 'regression' keyword.

These elements all have an explicit fixed width -- "width: 7.5em" -- which clearly may or may not be wide enough, depending on the width of the text (since we're using a variable-width font).

If I remove that fixed-width, and instead substitute in "font-family: monospace; margin-right: 2px", then I get EXPECTED RESULTS, pretty much.

(Alternately, if we don't want to use a monospace font, I think there may also be some CSS property that lets you ask for equal character spacing/positioning, without having to use a monospace font. I can't remember the name, but jfkthame probably does.)

Comment 3

[Jonathan French (:jfrench)]

This layout change occurred ~7 months ago to align resultset header bar contents when the job tables are collapsed.
https://github.com/mozilla/treeherder-ui-deprecated/pull/282

We no longer have a job collapse button, so we could possibly revert the layout change and let them be ragged again.

Curious Daniel are you running with non-default browser/OS font size?  I don't think other folks have seen this on Ubuntu since the change a half year ago, per your screen grab. I could be wrong though.

I haven't seen it on Win or OSX after the change. Trying right now altering my browser font or OS font sizes, I can't yet reproduce it.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Jonathan French (:jfrench) from comment #3) 
> Curious Daniel are you running with non-default browser/OS font size?

Nope.

Also, I can't repro in current Firefox release, so there may be a recent browser regression (or behavior-change) involved here.

Comment 5

[Jonathan French (:jfrench)]

Interesting, thank you for the update! I wonder if Chrome looks the same as Firefox release, or if it looks the same as Nightly on your system?

I had tried:

OSX 10.10.3
Firefox Nightly, Release
Chrome
Opera*
Safari*

*unsupported on Treeherder I checked anyway for the clipping and it was fine also

Win
Firefox Release

Comment 6

[Daniel Holbert [:dholbert]]

There is indeed a recent browser regression here -- mozregression directed me to the patch for bug 1165693, which indeed seems to have caused us to sometimes pick the wrong font on linux (bug 1173826).

Anyway -- I think it may still make sense to leave this bug open & fix TreeHerder to be more resilient here. In particular:
 - seems like we shouldn't be depending on precise assumptions about how many characters will fit into how many em's of width. If we intend to show the full 12-character csetID (and I think we do), we should size this element to its intrinsic size.
 - seems like we should be making these characters equally spaced, either via a monospace font or via whatever that CSS feature I'm remembering was, at the end of comment 2.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: screenshot before the gecko regression (bug 1173826)

For reference, here's a screenshot from before the gecko regression (bug 1173826). As you can see, there's no ellipsizing here, but the cset IDs are still different widths, which IMO makes them a little hard to read. [and can cause issues/inconsistencies if a different font is chosen, as described above]

Comment 8

[Jonathan French (:jfrench)]

Attachment: treeherder-PR662

Please see above PR for status and review.

Comment 9

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/c3d27c904192deafdbec558eb54011ef153f1256
Bug 1175991 - Use monospaced revisions for column readability

Comment 10

[Jonathan French (:jfrench)]

Marking fixed per above merge. Will verify on the next push to stage/prod.

Comment 11

[Jonathan French (:jfrench)]

I have a small nit to fix, the baseline text alignment is a bit out. So I will fix that.

Comment 12

[Jonathan French (:jfrench)]

Attachment: treeherder-PR686-supplemental

Supplemental tweak PR. It's so tiny I just checked with camd over irc rather than flagging for review.

Comment 13

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/671d23d01feaa70fc77a28d2f92dd8299b3b2d87
Bug 1175991 - Supplemental tweak for monospaced revisions

Comment 14

[Jonathan French (:jfrench)]

Verified fixed on production.