1144382 - title text in reading list sidebar item wraps poorly, leaving partial text visible

Status: VERIFIED FIXED

(Firefox Graveyard :: Reading List, defect, P1)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

Attachment: reading list sidebar screenshot

As shown in the second item in the reading list from the screenshot attachment the title is long and wrapping but wraps to a 3rd line which causes it to partially display.

This is a notorious CSS issue to which there are many hacks.

Comment 1

[Bryan Clark (DevTools PM) [:clarkbw]]

Also to note that the top of summary text should be aligned with top of preview image (currently text is a few pixels up too far on OS X) http://cl.ly/image/3z3W2J2Z3i36

Florian notes this might actually be the cause of having a few pixels of the 3rd line of text visible.

Comment 2

[Andrei Vaida [:avaida]]

This sounds like Bug 1143689.

Comment 3

[Florian Quèze [:florian]]

(In reply to Andrei Vaida, QA [:avaida] from comment #2)
> This sounds like Bug 1143689.

Let's fix the partially visible 3rd line here, and keep bug 1143689 for the missing ellipsis.

(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #1)
> Also to note that the top of summary text should be aligned with top of
> preview image (currently text is a few pixels up too far on OS X)
> http://cl.ly/image/3z3W2J2Z3i36
> 
> Florian notes this might actually be the cause of having a few pixels of the
> 3rd line of text visible.

This turned out to not be related, I'm (mostly) fixing this misalignment with the CSS changes in bug 1123525.

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch

There's currently a "2.8em" hardcoded height value. I suspect this value has been selected empirically to fit on Windows.

To show correctly 2 lines of text, we need to have twice the line-height. Unfortunately I don't think there's any CSS-only way to get that size, so using a value relative to em is probably our best approximation.

I looked at the (default) line height values and the font-size values on Windows 7, Ubuntu 13.10 and OS X 10.10.

'line height / font-size * 2' gives these values:
Mac: 2.45
Ubuntu: 2.32 (can be rounded down to 2.3 without visible difference)
Windows: 2.83 (can be rounded down to 2.8 without visible difference; I think that's where our current value comes from).

This patch is making that height value platform specific, using 2.45em, 2.3em and 2.8em for Mac, Linux and Windows respectively. It's not a real fix, but I think it will hide the problem almost everywhere.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8579619 [details] [diff] [review]
Patch

Review of attachment 8579619 [details] [diff] [review]:
-----------------------------------------------------------------

We handled something very similar to this for the Firefox menu, but we didn't need platform specific values there. Maybe we need to specify line-height? And you can use clip: rect(); to manually chose the clipping rectangle.

https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#201-202,212-213 for the Firefox menu panel CSS that does this there.

Comment 6

[Florian Quèze [:florian]]

Attachment: Patch v2

Fair enough, let's just set the line-height. I was reluctant to override the OS-specific system line height, but this has the advantage of also fixing the bug for non default system font sizes :).

Comment 7

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/d3923b8fe2fa

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d3923b8fe2fa

Comment 9

[Andrei Vaida [:avaida]]

Verified fixed on Nightly 39.0a1 (2015-03-23), using Ubuntu 14.04 (x64), Mac OS X 10.9.5 and Windows 7 (x64).

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/57a7834350dd

Comment 11

[Andrei Vaida [:avaida]]

Verified fixed on Aurora 38.0a2 (2015-03-29) as well, using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

Please note that this scenario is still affected by Bug 1143689, as there's still no ellipsis displayed for the item titles.