Open Bug 228593 Opened 21 years ago Updated 2 years ago

identity drop-down gets confused by shared identities

Categories

(MailNews Core :: Composition, defect)

defect

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: ch.ey, Assigned: ch.ey)

References

(Blocks 1 open bug)

Details

(Whiteboard: [patchlove])

Attachments

(2 obsolete files)

When sharing identities between accounts, the identities drop-down menu gets confused. The key in the value attribute is identity.key. If you're using an identity in multiple accounts, the popup entry selected will always the first one using an identity. So you have peter@example.org via newserver1.example.org peter@example.org via news.mozilla.org peter@example.org via news.blahblah.com Selecting news.mozilla.org in your folder pane, clicking Compose always sets you at peter@example.org via newserver1.example.org. That's the same for identites shared by mail accounts. But while it's a merely cosmetic problem there, it's a real one for news. That's because the outgoing NNTP server is bound to the account, not to the identity. So the user sees peter@example.org as From: and thinks it's the one of news.mozilla.org but it's newserser1.example.org. And the mail goes out through this ... So maybe we've to change the drop-down menu a little and build a combined key consisting of "idkey accountkey" to make it unique. That requires dealing with split(" ") when accessing the key in the value attribute but should be future-proof. The changes for building the drop-down (in FillIdentityListPopup(), MsgComposeCommands.js) and extracting it (in getCurrentIdentity()) is simple. My only problem is getting the accounts key into ComposeStartup()'s through the aParams parameter.
Blocks: 228597
Attached patch example patch (obsolete) (deleted) — Splinter Review
This is the js code to generate and handle the combined key. The code in ComposeStartup() assumes, the account will be provided in aParams. The function getCurrentAccountKey() is a modified version from bug 228597. That also shows that carrying the account key with us is also a good deed outside of this bug.
Blocks: 18906
Keywords: helpwanted
Attached patch proposed patch (obsolete) (deleted) — Splinter Review
This is again one of the patches doing nearly nothing except pushing the account through dozends of functions. All real work is described in comments #0 and #1. The cpp code is for providing the account in the params.
Attachment #137486 - Attachment is obsolete: true
Assignee: sspitzer → ch.ey
Status: NEW → ASSIGNED
Attachment #143327 - Flags: review?(mscott)
Christian, when we go to find the default identity for the compose window, we call getIdentityForServer which trys to pick the right identity to choose. This code enumerates over just the identities for the current server and then picks the right one to bring up the compose window with. Is something broken there that is causing us to look an identity for an account that is not even the currently selected account?
No, the problem isn't the code that looks up the identity. But if an identity is assigned to multiple accounts, its identity key is the value for multiple entries in the identity drop-down. So if an identity is selected (identityList.value = identity.key) it just chooses the first entry with this value (key). I tried to make the values uniqe by composing them from identity.key + account.key. It's a huge amount of code to change, maybe it can be done another way too. I'd be happy if so.
*** Bug 250364 has been marked as a duplicate of this bug. ***
Bug 250364 is a duplicate of this bug User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040705 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040705 Compose/Reply does not select the correct Identity for the currently selected account. It always defaults to the first account identity. For example I have 2 mail accounts. While in mail account 2 I select reply or compose. The compose comes up but the from address Identity is not the identity of account 2. It is the identity of account 1. In earlier versions (April/May 2004) this worked as expected. Reproducible: Always Steps to Reproduce: 1.Setup 2 or more mail accounts. Make sure the itentity (Name, from address, etc) are different. 2.Select compose or reply while in the second mail account. 3.Notice the from address at the top. It will default to the first account and not the account you are replying from. Actual Results: The from address is not the from address defined in the current account. Expected Results: The from address (identity) should have defaulted to the identity of the current account.
Jim, I read your other bug yesterday but couldn't find why it's a dupe to this one. If you don't share one identity (the id, not only the data) between your two accounts, it shouldn't be this bug. Additionally this bug exists since last December but you wrote April/May version work as expected. There's something wrong.
Reading this bug sounded like what I was experiencing. You are right that this bug is not exactly like the one I reported.
Product: MailNews → Core
Blocks: 228980
Christian Eyrich, is there a reason to keep this bug open? In the compose window now, the identity is appended with an account, which seems to address what you were talking about here. There are still problems with uniquely identifying identities -- for instance, I have two identities with the same username and email address associated with the same account, one of which uses a sig and the other doesn't, but they appear identically in the dropdown. But the issue of knowing which news-server will be used appears to be solved.
> Christian Eyrich, is there a reason to keep this bug open? I think yes. The issue may be theoretical for two reasons: 1. The very same identity (i.e. mail.identity.idx) can only be assigned manually, not via the UI. 2. The user has the chance to notice the automatically picked up identity is the wrong once since it's displayed in grey behind the address. But from what I see and tested, the problem is still valid.
QA Contact: esther → composition
Product: Core → MailNews Core
Christian, can you check that this patch still applies and request new review? I'll reset review request next. (I wonder if this is one of my current problems)
Whiteboard: [patchlove]
Comment on attachment 143327 [details] [diff] [review] proposed patch canceling mscott review
Attachment #143327 - Flags: review?(mscott)
As part of patchlove, David, is this 4.5 year old patch still relevant? (Btw, the patch has already bitrotted.)
Comment on attachment 143327 [details] [diff] [review] proposed patch patch has bitrotted.
Attachment #143327 - Attachment is obsolete: true
Flags: wanted-thunderbird3?
I assume it's per se valid, but since it's mainly a theoretical issue (you have to manually go into the prefs and make the identity shared - can't in the UI, as per comment 11), I don't think it's worth doing.
Whiteboard: [patchlove] → [patchlove] [wontfix?]
Definitely not WONTFIX, especially since the UI inability to create this is the actual problem. ;-) Just low priority.
Whiteboard: [patchlove] [wontfix?] → [patchlove]
wanted‑thunderbird3- as this is very much an edge case.
Flags: wanted-thunderbird3? → wanted-thunderbird3-
Christian, any chance of a new patch?
Not from me for the next months.
> "by shared identities" in bug summary. How can identity be shared, without manual modification of prefs.js? Identity is already created as slave of account always, even when news account. mail.account.accountA.identities = idB, ..., idZ Identity defines usable email adress as From:. mail.identity.idB.useremail = userid-B@mail.domain.name-B mail.identity.idZ.useremail = userid-Z@mail.domain.name-Z And any dentity can have any usermail value. So multiple identities of same email address is possible. However, if current Tb's UI is used for identity definition, "shared identities" is impossible. As Magnus says in comment #15, to force the impossible "shared identities", manual prefs.js change is currently needed. mail.account.accountX.identities = idZ mail.account.accountY.identities = idZ But this is manual corruption of prefs.js for current Tb. If protection from "shared identities by manual corruption of prefs.js" is needed, I think "remove double booking of identity at start up" is sufficient, as done on unused server.serverN to avoid incorrect account.accountN.server value.
No longer blocks: 228980
this bug should be closed, probably as fixed: the problem as described is: "a possible confusion in the identity drop down list when an identity has been manually shared between two or more accounts" it is impossible to be confused for three reasons: 1/ as said previously, the server name is now clearly displayed in grey next to the identity email/name 2/ the default identity selected is always related to what i'm responding to or what i'm composing: the default mail identity is selected if i'm composing or responding to a mail and the default newsgroup identity is selected if i'm composing or responding to a newsgroup, so, by default, even if the shared identity is the default one it will be selected accordingly to what i'm doing 3/ if i intentionally choose the wrong identity, the one linked to the mail account instead of the one linked to the newsgroup account and i try to send to a newsgroup, i will have an alert expliciting that i'm not using the "right" identity (and in the opposite case, newsgroup account identity to mail recipient, there is no difference with using the mail account identity since it is the same identity - same mail, same smtp, same data) what possibly remains is: (In reply to WADA from comment #20) > If protection from "shared identities by manual corruption of prefs.js" is > needed, I think "remove double booking of identity at start up" is > sufficient and (In reply to Mike Cowperthwaite from comment #9) > There are still problems with uniquely identifying identities -- for > instance, I have two identities with the same username and email address > associated with the same account, one of which uses a sig and the other > doesn't, but they appear identically in the dropdown. But the issue of > knowing which news-server will be used appears to be solved. as another bug since it is about two different but similar account
(In reply to roger21 from comment #21) > as another bug since it is about two different but similar account i meant identities not accounts
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: