1780909 - Characters with descenders (q, p, g, j, y) are cut off hence unreadable in Contact List

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect, P2)

Description

[Andreas]

Attachment: contact list.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.5060.114 Safari/537.36 Edg/103.0.1264.62

Steps to reproduce:

Have the density of Thunderbird set to "Compact"
Store a contact with the letters q, p, g, j, or y.

Actual results:

The letters are cut in the Contacts List both in the contact name and in the e-mail address.
I think this bug was introduced with version 102.0.3. Maybe it's related to the fix of the contact picture size.

Comment 2

[Thomas D. (:thomas8)]

Thanks Andreas, good bug report!
Didn't see yours when I filed mine today - bug 1781114.

(In reply to Andreas from comment #0)

I think this bug was introduced with version 102.0.3. Maybe it's related to the fix of the contact picture size.

Yes, most likely regressed by bug 1776078.

This isn't harmful, but it's pretty bad UX as it will make addresses with such characters all but unreadable (as seen in screenshot above) -> P2.
Alex, can you make a plan?

Comment 3

[Alessandro Castellani [:aleca]]

Attachment: Screenshot from 2022-07-25 12-00-48.png

Do you also have a larger font size?
I can't reproduce this unless I increase the font size a lot.
In the screenshot I have compact density and font size at 15px compared to the default 12px.
I think this is a mix of font sizing and font family.

From your screenshot also sizing and spacing seem different than the default. Do you have any userChrome customization or add-on changing the aspect of those items?

There's no "quick fix" for this, the only thing that can fix it is updating the core code of the treelistbox in order to set the height of each contact element based on the font-size, rather than having a fixed predefined height like we have right now.

This architectural change is scheduled for the future.

Comment 4

[Thomas D. (:thomas8)]

Attachment: Screenshot 3: Characters with descenders cut off in contacts list

102.1.0 (64-Bit), Windows 10

I have regular font size (used reset to make sure). View > Font size > Reset font size.
No customizations, no add-ons (verified with Trouble Shoot Mode).
So from my pov, this fails for the default case.

I have some Windows display scaling applied, but that shouldn't matter as it will scale everything equally.

Comment 5

[Thomas D. (:thomas8)]

Attachment: Screenshot 4: After using "Increase Fontsize" a lot, the layout breaks completely

And indeed, as hinted by Alex, after using "Increase Fontsize" a lot (to XXL), the layout breaks completely :-/

Comment 6

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #3)

Created attachment 9286936 [details]
Screenshot from 2022-07-25 12-00-48.png

Alex, is this a screenshot on Windows?
Why is the font white on yours?

Comment 7

[Alessandro Castellani [:aleca]]

That's on Linux, don't worry about the screenshot as that was taken when the window didn't have the focus, that's why the weird faint blue color.

So from my pov, this fails for the default case.

I see, I was able to reproduce this on macOS.
I'll see if I can find a quick fix, otherwise as I said, this will unfortunately be fixed later during this cycle.

Comment 8

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #3)

There's no "quick fix" for this, the only thing that can fix it is updating the core code of the treelistbox in order to set the height of each contact element based on the font-size, rather than having a fixed predefined height like we have right now.

Well, I ran mozregression and this was regressed by bug 1776078 D150214... So as the display has been correct all along before (haven't checked scaling, but the default case at least), maybe this can be rectified.

Comment 9

[Alessandro Castellani [:aleca]]

I think we can solve this by adding

ab-card-listrow .ab-card-second-line {
  line-height: 1.2em !important;
}

here: https://searchfox.org/comm-central/rev/12d3830dea34b6289efb9dad17496dac8c093ab1/mail/themes/shared/mail/aboutAddressBook.css#492

Thomas, can you confirm doing so fixes the issue on Windows and doesn't create any weird outcomes?

Comment 10

[Thomas D. (:thomas8)]

Attachment: Screenshot with bandaid fix applied

(In reply to Alessandro Castellani [:aleca] from comment #9)

ab-card-listrow .ab-card-second-line {
  line-height: 1.2em !important;
}

Thomas, can you confirm doing so fixes the issue on Windows and doesn't create any weird outcomes?

Tested on 102.1.0 (64-bit).

You also need to fix the name row which has same problem. 1.2 is not enough for me. 1.3 (without em) fixes this for me for the default size.
Font scaling still breaks the layout because of the fixed 36px height and fixed row positioning - current hard-coded implementation is so weird!

ab-card-listrow :is(.ab-card-first-line, .ab-card-second-line) {
  line-height: 1.3;
}

Comment 11

[Alessandro Castellani [:aleca]]

Micah, can you take care of this to create a patch with the changes suggested on comment 10?
Make sure the edit doesn't affect negatively the various density variations in the Address Book and it's not affecting Linux (and macOS if you can otherwise I'll test this during review).

Comment 12

[Alessandro Castellani [:aleca]]

Assigned is only for when a patch is present.

Comment 13

[Thomas D. (:thomas8)]

Attachment: Bug 1780909 - Adjust line-height to avoid cutting off descenders in AB results list. r=aleca

Comment 14

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #12)

Assigned is only for when a patch is present.

As discussed, in terms of bug management, we really want to preserve the literal meaning of the flag for bug searches etc. where Assigned == assigned to someone to work on it, regardless of patch presence. Which looks like the original intention and is much more helpful.

Of course everyone will be most happy if it's assigned and has a patch... so following my testing of Aleca's idea per my comment 10, let me just offer the patch - here it is! ^^ ;-)

We should try to fast-track this as much as possible because distorted characters in default AB view aren't nice at all, can actually make entries unreadable.

Comment 15

[[e-mail]]

Attachment: Bug 1780909 - Increase line-height in Address Book. r=Paenglab,thomas8

Comment 16

[Thomas D. (:thomas8)]

So per Aleca's subsequent comment, turns out that Micah was already working on this in an orphaned diff with the wrong bug number (now rectified) -> returning this to him. It's good to see action on this!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/89539b88e396
Increase line-height in Address Book. r=Paenglab,thomas8

Comment 19

[Wayne Mery (:wsmwk)]

micah, Is this good for uplifting to 102?

Comment 20

[[e-mail]]

Comment on attachment 9289076 [details]
Bug 1780909 - Increase line-height in Address Book. r=Paenglab,thomas8

[Approval Request Comment]
Regression caused by (bug #):
bug 1776078

User impact if declined:
Contacts containing letters with descenders will be much harder to read.

Testing completed (on c-c, etc.):
completed on c-c and beta

Risk to taking this patch (and alternatives if risky):
This patch only changes one css file so it shouldn't be too risky.

Comment 21

[Wayne Mery (:wsmwk)]

Comment on attachment 9289076 [details]
Bug 1780909 - Increase line-height in Address Book. r=Paenglab,thomas8

[Triage Comment]
Approved for esr102

Comment 22

[Rob Lemley [:rjl]]

Thunderbird 102.3.1:
https://hg.mozilla.org/releases/comm-esr102/rev/c778bbdd7d4a