1900211 - Wrong sender name shown for mailing list posts since 'Foo Bar via mailinglist' ends up in collected address as displayname

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect, P2)

Description

[Kai Engert [:KaiE:]]

Using Daily 128

When receiving email from a certain mailing list, Thunderbird displays the wrong "From" and "To" fields.
According to the name shown, it claims that I am the sender, but my name is not contained anywhere in the email, and my email address is contained in the Received: headers from MTAs, only.

I get this display:
From Kai Engert via mailinglist <[e-mail]>
To: Kai Engert via mailinglist

I have a theory what's happening.

Apparently, this mailinglist sometimes rewrites the From: email address, as it's common in some configurations to avoid DMARC delivery issues.
The mailing list software changes the "From:" field of the email, and replaces the original email address with the mailing list address, but it also adds the original sender "name" plus the word "via" plus "name of mailinglist".

I assume that at the time Thunderbird first saw an email from that mailinglist (which apparently was an email that was from myself),
Thunderbird collected [e-mail] into the address book "collected addresses".

When it collected that address, it also assigned a display name.
Because that email was sent by me, it associated the name "Kai Engert via mailinglist" with the email address "[e-mail]".

(I confirmed that I have such an entry in my address book.)

The email I received today has this From: header
From: some-other-name via [e-mail] <[e-mail]>

Apparently Thunderbird decided to NOT show the name from this specific email, but decided to use the name from the address book instead,
which is wrong and very misleading in this scenario.

Comment 1

[Alessandro Castellani [:aleca]]

Indeed, there's an underlying issue on how Thunderbird collects addresses and tries to "smartly" fill up the Display Name field.
This behaviour has always been present but only recently is more obvious since we finally fix the display of known VS unknown addresses in the message list in bug 243258.
We also added a preference in the settings to control the display settings.

I think Display Names shouldn't be pre-populated for collected addresses if the message comes from a mailing list.
Do we have a fail proof way to detect this scenario?

Comment 2

[Magnus Melin [:mkmelin]]

Probably https://searchfox.org/comm-central/rev/9154a515faba4ae8533e85edd2b9938bc0f361d2/mailnews/compose/src/MessageSend.sys.mjs#1190 should check compType Ci.nsIMsgCompType.ReplyToList and probably blank out displayName?

Comment 3

[Alessandro Castellani [:aleca]]

Ben, can you take care of this?

Comment 4

[Ben Campbell]

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

Ben, can you take care of this?

I can investigate, but you'd likely be better off with someone familiar with the code. I know next to nothing about compose and addressbook.

Comment 5

[Kai Engert [:KaiE:]]

Suggested first step: Define the intended behavior.

In which scenarios should the stored display name get ignored, and the name from the current message be shown?

Should the fix consist of two changes?

• ignore stored display name in certain situations
• don't save he display in address book name in certain situation

If we want both of the above, do we also want automatic cleanup of existing wrong entries,
or do we accept that users of previous Daily/Beta versions must clean up manually?

Comment 6

[Geoff Lankow (:darktrojan)]

(In reply to Kai Engert (:KaiE:) from comment #0)

I assume that at the time Thunderbird first saw an email from that mailinglist (which apparently was an email that was from myself),
Thunderbird collected [e-mail] into the address book "collected addresses".

Close, but not quite. You must've written to somebody via somelist <[e-mail]> and that's when the address and display name entered you address book. (The "Automatically add outgoing email addresses to" setting controls this.) So Magnus's suggestion in comment 18 seems like a good idea.

(In reply to Kai Engert (:KaiE:) from comment #5)

If we want both of the above, do we also want automatic cleanup of existing wrong entries,
or do we accept that users of previous Daily/Beta versions must clean up manually?

The latter. I don't think we should go through users' address books changing things.

Comment 7

[Geoff Lankow (:darktrojan)]

Additionally:
The time to remove the display name is when composing a new message. And in fact the Reply List button already does this. The ordinary Reply button doesn't, but maybe it should if the address matches the list address.

Comment 8

[Geoff Lankow (:darktrojan)]

In fact there's a preference which appears to do that: mail.override_list_reply_to. I'm not quite sure if that's the intent of bug 1382371 which put it there, but it seems to use the List-ID name instead of the From name when clicking on Reply [All].

Comment 9

[Kai Engert [:KaiE:]]

(In reply to Geoff Lankow (:darktrojan) from comment #6)

You must've written to somebody via somelist <[e-mail]> and that's when the address and display name entered you address book. (The "Automatically add outgoing email addresses to" setting controls this.)

You're right, I indeed have such a message in my sent folder.

I had viewed a message from the list, which has this to value:
"To: Kai Engert via mailinglist <[e-mail]>"

I had replied to this message in the past.

I just tried to repeat the scenario with the latest pre-128 code.
I deleted the old entry from the collected addresses address book.

If I reply all, the message I'm composing also says "To: Kai Engert via mailinglist ..."

I think that confirms your theory.

So Magnus's suggestion in comment 18 seems like a good idea.

Comment 18? Did you intend to refer to comment 2 from this bug?

Comment 10

[Kai Engert [:KaiE:]]

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

Probably https://searchfox.org/comm-central/rev/9154a515faba4ae8533e85edd2b9938bc0f361d2/mailnews/compose/src/MessageSend.sys.mjs#1190 should check compType Ci.nsIMsgCompType.ReplyToList and probably blank out displayName?

Thanks for your suggestion. Good idea, but it's probably better to do it earlier, and also avoid that the outgoing email uses the displayname.
I'll check if I can find the place.

Comment 11

[Kai Engert [:KaiE:]]

Ok, I should read Geoff's messages, he already gave additional thoughts in comment 7 and 8.

Comment 12

[Kai Engert [:KaiE:]]

Further complication:

The message I was replying to does NOT have standard mailing list headers, like "List-ID:"

Because ither messages to that list DO have the header, I assume the reason for the missing header is:
I was also explicitly on CC of that message.

The message that was delivered into my mailbox was apparently the directly delivered copy because of the CC.

When I reply to that message, I do not get a "reply to list" button.
I have a reply and a reply all button.
The reply all button produces the "TO: Kai Engert via"

I assume that means we cannot solve the problem by checking for the ReplyToList flag?

Comment 13

[Kai Engert [:KaiE:]]

I looked at other messages from that list, which contain the name of some other person,
like "To: other person via mailinglist ..."
and in which I was NOT on CC.

When replying to that message, a direct reply works fine, a reply list works fine (only the email address is used, not the displayed name).

However, when using "reply all", I get the "other person via mailinglist..." in To: of the outgoing message.

Comment 14

[Kai Engert [:KaiE:]]

It's possible that this is NOT a regression in 128.

Using another profile, where I still use 115, I made sure that I don't have the list address in my address book.

When using reply all with 115, I get the same behavior. Message composer also shows "To: Kai Engert via mailinglist ..."

I didn't test actual sending (I don't want to spam that list), but I assume that would also result in the wrong collected address.

Comment 15

[Kai Engert [:KaiE:]]

Based on the additional findings, I don't know if we could fix this at the time we're collecting addresses or composing a reply to all. Please let me know if you have an additional idea.

But regardless, I still think we should try to fix it at the time we're displaying a message.

Suggestion:
If a received message contains a List-Id: header
(or other identifiers that make it clear that it's a mailing list, not sure if those exist)
then:

• Ignore the display name found in the address book
• display the From: information that is found in the current message

Comment 16

[Kai Engert [:KaiE:]]

Another scenario, which I think supports my suggestion from comment 15.

My address book contains an entry for the thunderbird planning mailing list.
The display name is set to "Thunderbird Planning".

When receiving emails from that list, I cannot see the real sender.

Despite the "From:" field containing the name of a person, Thunderbird doesn't show it, neither in the "From" header section, nor in the "From" column.

Comment 17

[Kai Engert [:KaiE:]]

The scenario described in comment 16 is a regression.

With TB 115, I get the display name from the "From" header shown in all three places.

Comment 18

[Kai Engert [:KaiE:]]

If I toggle the preference "show only display name for people in my address book",
I get the following behavior:

With the above setting enabled, I get display name "Thunderbird Planning", only
(which is the name from the address book entry).

With the above setting turned off, I get display name "Real name via Thunderbird Planning <[e-mail]>"

This seems wrong.

Should the same display name be shown in both scenarios?
Shouldn't the action of the pref be limited to switching off the display of the email address?

My expectation is that "Real name via Thunderbird planning" should be shown in both scenarios,
either with or without the email address.

Comment 19

[Kai Engert [:KaiE:]]

So it seems the full meaning of
""show only display name for people in my address book"

is:
"For email addresses that are contained the address book, don't show the email address, only show the display name, and don't show the display name from the email, but rather show the display name that is stored in the address book."

For mailing list addresses, which the user has already sent to, this will result in automatic collection and showing the list name, not the sender name, for all messages that use a rewritten From address by the mailing list software.

This will be the default behavior, if I understand correctly, and affect everyone using mailing lists.

So, do we want to provide a fix, by changing the implementation, and even if the pref is set, show the "From" header display name for all messages that are apparently sent by a mailing list,
or do we expect users who wish to avoid this potential confusion to turn off this pref?

Comment 20

[Kai Engert [:KaiE:]]

I see the default value for mail.showCondensedAddresses is false,
which is the configuration that prefers to show the header display name, apparently.

I don't understand why my profile had this pref set to true.
I'm taking back my concern regarding the default behavior.

I no longer claim the wrong "From" display name is a regression,
because the profile that I use with version 115 had this pref set to false,
and setting it to "true" gives me the same behavior in 115.

Comment 21

[Alessandro Castellani [:aleca]]

Redirecting this to Magnus so he can tackle this as per his recommendation in comment 2.
This is a bit outside Ben's comfort zone.

Comment 22

[Corey Bryant]

Magnus, can you adjust the severity and priority based on your understanding of this issue?

Comment 24

[Eliot Lear]

Please see comment 6 of bug 1913411 for a proposed general approach to address this bug.

Comment 27

[andré]

I reported the same problem in Bug 1930539.
In my case, there are several mailing lists @ml.mageia.org.
Each mailing list has a certain prefix, for example dev , qa-discuss , discuss , etc
For each mailing list I am subscribed to, there is one entry in my adresse book with the prefix of the mailing list.
It is these entries which replace the actual sender of the email.
eg., for [e-mail]
it displays the sender as the entry in my adresse book,
"David W. Hodgins" [e-mail]
even though that user had few or no posts in the discussion in question.

For several other Mageia mailing lists, the name displayed corresponds to the entry in my adresse book for the particular mailing list, and not the actual sender.

These symptoms occured with the option "show only the name of those in the adresse book" checked. (The default)
When the option was unchecked, the correct poster is displayed on each post when a discussion is expanded.
If the discussion is collapsed, the error still occurs for the posts in the collapsed list, but not for the title of the discussion.
This seems to correspond exactly to the initial post in this thread.

Comment 28

[Magnus Melin [:mkmelin]]

Attachment: Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

Comment 29

[Magnus Melin [:mkmelin]]

Attachment: Bug 1900211 - Re-implement addresscollection. r=#thunderbird-back-end-reviewers

We'd always call the collectSingleAddress now.
I removed collection of chat screennames, all of which are defunct now anyways.
The old code did was pointlessly complicated. This new code is also much faster than the old code (10x to 100x depending on how many addresss)
For 50k emails I get 0.02692ms per email. Old code choked a bit on that many, but for 5k emails I got 2.9834ms per email

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/14c7a3f60a5c
Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=aleca
https://hg.mozilla.org/comm-central/rev/3abf248137b6
Re-implement addresscollection. r=john.bieling

Comment 31

[Magnus Melin [:mkmelin]]

Comment on attachment 9441485 [details]
Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): strictly not regression, but aggravated by changed address display handling in 128
User impact if declined: may get confused about who they are sending to
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): safe
(Can uplift both patches, or just the first one.)

Comment 32

[Corey Bryant]

Comment on attachment 9441485 [details]
Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Comment 33

[Corey Bryant]

Comment on attachment 9441485 [details]
Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

[Triage Comment]
This will be included in today's merge of cc->comm-beta.

Comment 34

[Corey Bryant]

Comment on attachment 9441485 [details]
Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr128

Comment 35

[Daniel Darnell [:dandarnell]]

This needs an ESR128-specific patch.

Comment 36

[Ben Bucksch (:BenB)]

Attachment: Cleaner access of addr.name

Patch on top: Accept addr.name being null and to be safer, and also to be cleaner in the code, because displayName was already declared above, so might as well use it.
Credits: BetterBird

Comment 37

[John Bieling]

Hi Ben! I know it takes extra steps, but please submit this patch via phabricator. It simplifies the process for all involved parties (including the landing process). It is a one time setup an it will make it easier for you as well.

../mach install-moz-phab

This will ask for an API key from Bugzilla and gives you a link on how to get that. Once you are logged into Bugzilla, copy it and paste it into the console.

Once installed, and you have a single hg changeset on top of the last public changeset, you can execute

moz-phab

Which will submit your patch. In order to update your patch later, amend all changes into the changeset and call moz-phab again. You need to make sure to not remove the last line from the commit summary, which reads something like

Differential Revision: https://phabricator.services.mozilla.com/D232068

This line is identifying which phab should be updated. If no such line is present, moz-phab will create a new phab, instead of updating the old one. To review the patch, we can simply execute (as an example)

moz-phab patch D232068

to download the patch onto our systems. Also, the review itself is much more simple if we can use Phabricator. Once the thing has been approved, it can be merged using Lando. Our Sheriffs documents no longer state how to handle bugzilla patches and our new staff would not know how to land this.

To make it easier for everyone involved, I kindly ask you to re-upload the patch to Phabricator.

Comment 38

[Magnus Melin [:mkmelin]]

And in a new bug please. Also explain how that can be null. MailServices.headerParser.parseEncodedHeader("[e-mail]")[0].name; is ""

Comment 39

[Magnus Melin [:mkmelin]]

Please back out for 135. There's some refining to do.
https://hg.mozilla.org/comm-central/rev/14c7a3f60a5c
https://hg.mozilla.org/comm-central/rev/3abf248137b6

Comment 40

[Daniel Darnell [:dandarnell]]

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

Please back out for 135. There's some refining to do.
https://hg.mozilla.org/comm-central/rev/14c7a3f60a5c
https://hg.mozilla.org/comm-central/rev/3abf248137b6

Backed out.

Comment 41

[Corey Bryant]

Comment on attachment 9441485 [details]
Bug 1900211 - Don't collect displayName for mailing list posts since 'Foo Bar via mailinglist' in address book is easily confusing. r=#thunderbird-reviewers

[Triage Comment]
Removing approval for 128 for now