317167 - Reply and Reply-all on messages sent "From: myself" should consider all account identities when setting To field

Status: RESOLVED FIXED

(MailNews Core :: Composition, enhancement)

Description

[Magnus Melin [:mkmelin]]

See bug 85296 comment 19. 

Reply and Reply-all on messages sent "From: myself" should consider all account identities when setting To field.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

also cleans up a warning an some extra whitespace

Comment 2

[David :Bienvenu]

Comment on attachment 203731 [details] [diff] [review]
proposed fix

this seems to me to cast too wide a net of identities - it uses all the identities on all the servers that use the current identity. I think the following approach might be more precise:

See what account the message came in from. If we find that, just check the identities for that account (that only works for pop3 messages, I believe). If we can't find the account, check the server of the folder the message is in, and use the identities for that server (which in general is the imap case)

What do you think? My suggestion would miss the case where an imap message is moved to an other account, but yours might too, but mine would avoid more false positives. 

Here's some pseudo code, w/o error checking:
msgHdr->GetAccountKey(getter_Copies(accountKey));

if (!accountKey.IsEmpty())
{
 am->GetAccount(accountKey, getter_AddRefs(account));
 account->GetIdentities(getter_AddRefs(identities));
}
else
{
 msgHdr->GetFolder(getter_AddRefs(folder));
folder->GetServer(getter_AddRefs(server));
am->GetIdentitiesForServer(server, getter_AddRefs(identities));
}

Comment 3

[Aleksey Nogin]

I would like to request that there is at least an option to do this for all available identities.

Background: as a workaround for bug 166293, I have two different accounts for the same IMAP server, so messages I am replying to may "randomly" belong to a different account.

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

This patch addresses the review comments, I hope. It also introduces a boolean pref "mailnews.reply_to_self_check_all". When set to true, all available  identities are considered.

Comment 5

[David :Bienvenu]

Comment on attachment 205548 [details] [diff] [review]
proposed fix, v2

this looks good - I'll try running with it when I'm back from vacation on Wednesday, and if it works ok, I'll check it in. One thing we should add is a default value for the new pref in mailnews.js...

Comment 6

[David :Bienvenu]

Attachment: fix as checked in.

I fixed the braces style, and changed the name of the pref slightly, and added it to mailnews.js

Comment 7

[David :Bienvenu]

fix checked in, thx!

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 206309 [details] [diff] [review]
fix as checked in.

Sister bug 85296 is on the branch, time for this one too?

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

*** Bug 209329 has been marked as a duplicate of this bug. ***

Comment 10

[Scott MacGregor]

Comment on attachment 206309 [details] [diff] [review]
fix as checked in.

yup, time for this one too.