The symptom of this bug is the same as bug 54911 had. But this time only profiles are affected where multiple news accounts share identities. I knew this could happen with my fix (see bug 54911, comment #82), but thought it's a reasonable drawback for fixing the bug affecting everyone. Now after some people reported this problems I looked again at the code and found a better solution for the problem which is multiple-identity-proof.

This patch fixed the problem by carrying the account key in the drop down menu and getting the server from the account key instead the identity. Most of the code consists of pushing the account key through the back end to the place where it's usable (GetNntpServerByAccount()). I also found it necessary to save this key in Drafts and Unsent Messages in the X-Account-Key field (analog to X-Identity-Key) to have it ready when editing or send later. Please note that a fix of bug 228593 would make it necessary to slightly change FillIdentityListPopup() and getCurrentAccountKey().

1st thing - I think you'll need to make the corresponding change to thunderbird or it will break. Instead of passing both the identity and the account key, could you get the identity from the account? Or might the identity be different from the account's default identity?

Thunderbird changes, hm, don't know anything about what's different. I'll have to look. Of course it would be nice to get the identity from the account. But this relation isn't unique as it isn't from identity to account. An account can have multiple identities - since bug 226580 also in practice.

here's the thunderbird file: mail\components\compose\content\MsgComposeCommands.js mailnews\mapi\mapihook\src\msgMapiHook.cpp would need to be changed as well.

> mail\components\compose\content\MsgComposeCommands.js Ok, changed. But while we have gMsgCompose.SendMsg(msgType, progress); in Mozilla's file there's gMsgCompose.SendMsg(msgType, msgWindow, progress); in TB's. So is there another nsMsgCompose::SendMsg() for TB with this additional argument? I can't find such one under mail. > mailnews\mapi\mapihook\src\msgMapiHook.cpp would need to be changed as well. Sorry, I can't see why this file needs to be changed.

in TB, I'm referring to this line: http://lxr.mozilla.org/mozilla/source/mail/components/compose/content/MsgComposeCommands.js#1730 In mapi, http://lxr.mozilla.org/mozilla/source/mailnews/mapi/mapihook/src/msgMapiHook.cpp#401 Could you look again? Those both look like calls to the method you're adding a parameter to. I might be confused since your diffs aren't against the tip of the trunk...

>in TB, I'm referring to this line: > >http://lxr.mozilla.org/mozilla/source/mail/components/compose/content/MsgComposeCommands.js#1730 Yes, as I wrote, I found and changed it. That's not the problem. But the call from JS goes to a SendMsg() with the extra msgWindow parameter in contrast to Mozilla's nsMsgCompose::SendMsg(). I'm not sure where this SendMsg() function is. >In mapi, > >http://lxr.mozilla.org/mozilla/source/mailnews/mapi/mapihook/src/msgMapiHook.cpp#401 Oh, yes. Thanks. That looks not so good - e.g. in CMapiImp::Login() the identity key is being retrieved from the username (using nsMapiHook::VerifyUserName()). There's no unique connection to the account for me. I'll have another look on that tomorrow.

As I said, your diffs are not against the tip of the trunk. Here's what the tip of the trunk idl for that method looks like in nsIMsgCompose.idl void SendMsg(in MSG_DeliverMode deliverMode, in nsIMsgIdentity identity, in nsIMsgWindow aMsgWindow, in nsIMsgProgress progress); and here's the corresponding .cpp function: NS_IMETHODIMP nsMsgCompose::SendMsg(MSG_DeliverMode deliverMode, nsIMsgIdentity *identity, nsIMsgWindow *aMsgWindow, nsIMsgProgress *progress)

> As I said, your diffs are not against the tip of the trunk. Here's what the > tip of the trunk idl for that method looks like in nsIMsgCompose.idl [...] Oh oh, now I see. I guess a classic case of being a blockhead. My diff is not only three weeks old it also hasn't been made with -r HEAD. Sorry for all the fuss. So there's only the problem with the Mapi code.

as far as mapi's concerned, it's not a big deal re news. Anyway, the account for mapi is, by definition, the default account.

This patch contains the necessary changes for TB too. I also changed the call for SendMsg() from Mapi. The account parameter is nsnull - this should be sufficient since we're going to use the default account then. That should be ok according to comment #10.

Comment on attachment 139195 [details] [diff] [review] patch v2 we should ping the enigmail folks and let them know bout this change. I suspect some of the API changes will effect how enigmail works.

Ok, I'll do that. Would be nice if one of you could check this in, thanks.

mailnews/import didn't build when I applied the original patch, so I had to make these diffs to get it to build (Christian probably isn't building import...)

Comment on attachment 140680 [details] [diff] [review] fix to make import directory build I warned Patrick Brunschwig about this checkin and it turns out Christian already warned him and Patrick has fixed it for the next version of enigmail.

Yep, I mentioned that in comment #13.

Comment on attachment 140680 [details] [diff] [review] fix to make import directory build Oops, I'm indeed not building import.

fix checked in, thx, Christian.

I think there's another bug related to this one: If I share an identity with multiple accounts, and if I remove one of these accounts, it seems that other accounts are also deleted.

This has nothing to do with the bug here; please create a new bug for the error you found (or look for a bug with this topic)