Closed Bug 1569129 Opened 3 months ago Closed 3 months ago

mail notification oddly formatted with very long From: address

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set

Tracking

(thunderbird_esr6868+ fixed, thunderbird69 fixed, thunderbird70 fixed)

VERIFIED FIXED
Thunderbird 70.0
Tracking Status
thunderbird_esr68 68+ fixed
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: jorgk, Assigned: mkmelin)

References

(Regression)

Details

Attachments

(3 files, 2 obsolete files)

Attached image bad-notification.png

+++ This bug was initially created as a clone of Bug #1568109 +++

See attachment. Richard, can you see where this goes wrong?

Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)

It seems, that it doesn't recognize that this is a vbox and use a hbox instead.
Jörg, is this reproducible? Always from same sender?

Flags: needinfo?(richard.marti)

This is a little puzzling. I've just started TB and the notification for a message from a different sender was fine. So thinking about it, it could be that it only affects notifications for messages from that Yahoo mailing list. Where do you see a vbox/hbox? Surely the formatting doesn't depend on the content that's formatted??

It should be this vbox: https://searchfox.org/comm-central/source/mailnews/base/content/newmailalert.xul#27.

Yes, content should not be able to format this.

Yes, but why is that vbox filled badly? That happens somewhere in newmailalert.js, no? Basically the entire thing is a vbox, with some hbox(?) children, each one containing information about one message.

This is JS area, so better Magnus takes a look.

Well, the action happens here:
https://searchfox.org/comm-central/search?q=_createFolderSummaryMessage()&case=true&regexp=false&path=

So somehow the boxes aren't glued together/layed out correctly. No special JS skills needed. As you can see, we create another vbox with hbox children and some other XUL stuff.

(Notifications historically go in MW FE)

Component: Untriaged → Mail Window Front End

I've observed it for a while. Only the notifications from the Yahoo mailing list are badly formatted.

Actually, what is your expectation on how it should look? This "From" is unusually long which makes it look slightly odd, but other than that...

Flags: needinfo?(mkmelin+mozilla)

Actually, you are right ;-)

The from on that list is, for example (redacted):
Mariana Sol Cubertina mariana89889@hotmail.com [Barcelona-Freecycle] <Barcelona-Freecycle-noreply@yahoogroups.com>

But the funnily formatted notification never occurred to me in TB 60. Hard to check now without going back to TB 60 on my production profile. Maybe the From: was truncated before? Or an ellipses added? CSS can do that. Did that change?

Summary: mail notification not properly formatted → mail notification oddly formatted with very long From: address

It can (text-overflow: ellipsis;) but then the boxes need to have given widths. I don't think we used to do that.

I tested with 60 now - you can easily test it by sending yourself a mail with a very long custom from. It does indeed behave differently: only one line for this info.

Assignee: nobody → mkmelin+mozilla

Glad I convinced you. When upgrading to a new version, some things just cause a "huh?" effect without being able to pin down exactly what has changed.

Hmm, figured it out, but... 60 doesn't actually handle long from very nicely either: it can push the subject totally out of view and that looks bad. Also, comparing the behaviours I find myself missing the wrapped long subjects, typically all bugmail for instance.

The crux here is that by label value="something" doesn't wrap, but label with text content does wrap.

I've made the sender use label value, so that won't wrap anymore, and cropping with ellipsis will take place if the from is too long. The subject is kept as textContent, so if it's long then it does wrap.

We can't use a max-width in combination with flex="1" - that makes the notification the wrong size, so I've set fixed sizes for subject and sender.

The platform specific css files were almost identical, so moved to a shared file.

Attachment #9081887 - Flags: review?(jorgk)
Status: NEW → ASSIGNED
Comment on attachment 9081887 [details] [diff] [review]
bug1569129_notification_formatting.patch

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

::: mail/base/content/foldersummary.js
@@ +138,5 @@
>          if (this.showSender) {
>            let addrs = MailServices.headerParser.parseEncodedHeader(
>              msgHdr.author, msgHdr.effectiveCharset, false);
>            let folderSummarySender = msgBox.querySelector(".folderSummary-sender");
> +          folderSummarySender.value =

What's the difference between setting textContent and value?

Per comment 14, when you set value the content is non-line-wrapping (i.e. strictly one line).

Attached image 2019-07-31 10_52_44-.png (obsolete) —

Something totally wrong now. While you fix it, please add a comment why we use value in one case and textContent in the other.

Attachment #9081887 - Flags: review?(jorgk)

Did you rebuild? I'd imagine that's what it looks like without picking up the shared css.

Ah, I see the jar change didn't make it into the patch

Re. comment 19: I work here since 2015 ;-)
Re. comment 20: That's what I was going to say.

Will deal with the preview text in bug 1568109

Attachment #9081887 - Attachment is obsolete: true
Attachment #9081893 - Attachment is obsolete: true
Attachment #9081897 - Flags: review?(jorgk)

Looks good now.

Comment on attachment 9081897 [details] [diff] [review]
bug1569129_notification_formatting.patch

Thanks, and sorry for not reading the comment about value vs. textContent properly.

I'll set the uplift flags for you.
Attachment #9081897 - Flags: review?(jorgk)
Attachment #9081897 - Flags: review+
Attachment #9081897 - Flags: approval-comm-esr68?
Attachment #9081897 - Flags: approval-comm-beta?

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/627a555b3165
make mail notification properly formatted for long senders, and make sure long subjects also are properly shown. r=jorgk DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0

(In reply to Magnus Melin [:mkmelin] from comment #14)

The platform specific css files were almost identical, so moved to a shared file.

An improvement could be to move the line in jar.inc.mn below the #ifndef XP_MACOSX because Mac doesn't use this notification.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/de1e979e4910
Follow-up: Don't include shared/newmailalert.css on Mac. r=jorgk DONTBUILD
Attachment #9081897 - Flags: approval-comm-esr68?
Attachment #9081897 - Flags: approval-comm-esr68+
Attachment #9081897 - Flags: approval-comm-beta?
Attachment #9081897 - Flags: approval-comm-beta+

Finally I could see the effect in real life with the mailing list in question. Looks good now.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.