Closed Bug 226580 Opened 21 years ago Closed 21 years ago

fix backend multiple identity support

Categories

(SeaMonkey :: MailNews: Account Configuration, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: alecf, Assigned: alecf)

References

Details

Attachments

(1 file, 2 obsolete files)

Back when I made multiple identities, I never did the final work of parsing out
the mail.account.idX.identities array.. duh! 

thanks to david greaves for pointing this out.. I've got a fix and am attaching
a patch momentarily
Attached patch parse out the identity keys (obsolete) — Splinter Review
.. and here it is. Now I just need some reviewers.
Comment on attachment 136175 [details] [diff] [review]
parse out the identity keys

..its been so long, I forget who needs to review this..
Attachment #136175 - Flags: superreview?(mscott)
Attachment #136175 - Flags: review?(sspitzer)
Attachment #136175 - Flags: review?(sspitzer) → review+
OS: Windows XP → All
Hardware: PC → All
Summary: fix backend multiple identity support → fix backend multiple identity support
oops meant to add that the sr is for post 1.6b freeze :)
Same as bug 48757?

Would be *really* great, if we had working support for multiple identities, even
without account management UI.

I tried the patch and it works great for me (with 1.4.1), works as expected.
Thanks a lot, finally a bit cleanup in my accounts! :-)
Unfortunately, the account manager destroys the glory, open it, close it, and
your multiple identities are gone :-(, so don't use it. Creating a new account
using the UI (File|New|Account) seems to work, though.
Blocks: 226585
thanks for testing this, ben. sounds like we're not writing out the excess
identities. hmm... I'll continue investigating.
Will fixing this bug include some UI? Either way, could a *brief* pointer of
where/how to set up multiple identities be posted in this bug, so we (those
CC'd) can know and explain it to others?
> Will fixing this bug include some UI?

No, that's what "backend" means.

> Either way, could a *brief* pointer of
> where/how to set up multiple identities be posted in this bug

Bug 48757 (see above) and
<http://www.mozilla.org/mailnews/arch/accountmanager.html>. Further questions
about that please to the newsgroup.
No - no UI changes come out of this bug. That would be bug 44863.

Look at http://www.mozilla.org/mailnews/arch/accountmanager.html for some low
level information. Until we get UI, maybe someone can write up a nice doc on
this stuff.
Status: NEW → ASSIGNED
ok, I figured out the problem. Its not actually the opening or closing of the
account manager that blows it away.

the problem is that nsMsgAccount::AddIdentity() rewrites the pref every time.
That worked fine when there was only one identity, but now I have to figure out
a smart way to add the identity without upsetting the pref.

I think I need to make an AddIdentityInternal() or something, that adds the
identity without modifying the pref.

Patch forthcoming.
Alecf, I already filed bug 226585 about the account manager problem. Do you want
to deal with that problem there or keep it all here?
ok, here's an updated version. I switched to using nsCRT::strtok, because we
use that all over. I basically ripped off the pattern of using that and
StripWhitespace() from other places in the code.

Someone want to try this patch before I go to get it reviewed again? I won't be
able to test this until later today...
Attachment #136175 - Attachment is obsolete: true
Attachment #136175 - Flags: superreview?(mscott)
With the first patch, I noticed that quitting the app also removes the added
identities.

This is fixed with the second patch, as well as the account manager problem.
Thanks :-).

I now have a strange problem, though, with both patches. If I start from KDE's
menu, using KDE su, I don't see any identity (only "<>") for the account with
multiple identities. If I start from a shell (under that OS account), it works.
No idea what the reason is, might be something on my end.
oh, and yeah I'd rather just deal with it here - get the right patch in all at
once (especially since I now have the right patch)

No longer blocks: 226585
*** Bug 226585 has been marked as a duplicate of this bug. ***
Second patch works great! Steps to create two additional identities: add

   user_pref("mail.identity.id11.fullName", "My Name");
   user_pref("mail.identity.id11.useremail", "user1@do.main");
   user_pref("mail.identity.id12.fullName", "My Name");
   user_pref("mail.identity.id12.useremail", "user2@other.do.main");

and append both ids to your main account

   user_pref("mail.account.account1.identities", "id1,id11,id12");

in prefs.js. You may also add fcc_folder, sig_file etc.

However, creating a new _account_ with the existing UI doesn't work anymore.
Clicking on "Finish" in the Account Wizard crashes Thunderbird...
oh! ack. I'll look into that. thanks a ton for testing..
my bet is that the crash is fixed and doesn't relate to alecf's work, when you
report crashes (even of modified builds) it helps to indicate your build id.

i could be wrong, and if you crash w/ a build from today then i am wrong, a
stack would be good (even if it's just module names, my bet is that your crash
was nspr<appshell)
> my bet is that the crash is fixed and doesn't relate to alecf's work, when
> you report crashes it helps to indicate your build id.

"Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031125
Thunderbird/0.4a" build from CVS, checkout started today at 09:30 +0100 (CET).
It crashes only with the patch applied, a build without it does not crash.
I wrote:
> I now have a strange problem, though, with both patches.

I was indeed confused, the KDE menu entry started an unpatched build, duh.
It seems that unpatched builds don't show *any* identity for an account, if
there are several ones.

I can confirm (using a 1.4 branch seamonkey build with the second patch) that
creating a new account crashes the app. This didn't happen with the first patch,
IIRC.
ok, the previous patch had two flaws in it
1) it would pass empty identity key strings to nsCRT::strtok(), which would
assert and crash on an empty string. Fixed.
2) It would fail to save the identity key of new accounts. I had mixed up
"newIdentityKey" with "identityKey" towards the end.

I tested composition with mulitiple identities, and created some accounts,
without any problems.

I'm now looking for reviewers.. this patch should be ready to go. Anyone want
to test it some more too?
Attachment #136237 - Attachment is obsolete: true
Comment on attachment 136801 [details] [diff] [review]
parse identity keys, and really save them correctly

Here's the updated version of the patch, hopefully the last one.. scott, do you
mind reviewing? obviously this would be for moz1.7
Attachment #136801 - Flags: review?(mscott)
Target Milestone: --- → mozilla1.7alpha
Patch is working for me without problems! I think it's ready for check in :-)
oh, also see bug 18906, which is about automatically choosing your identity
based on the currently displayed message. This becomes more relevant when we
have multiple identities per account working.
Comment on attachment 136801 [details] [diff] [review]
parse identity keys, and really save them correctly

plussing for 1.7 trunk. 

I may be more agressive about taking this into thunderbird on a private branch
assuming I can get some UI cribbed up.
Attachment #136801 - Flags: review?(mscott) → review+
I feel pretty good about the patch, I guess I'd be slightly concerned about
callers of nsIMsgAccount::GetIdentities() in case they are counting on just one
identity. But it would probably be easy just to find those by search/inspection.
*** Bug 33384 has been marked as a duplicate of this bug. ***
Alec, just so I understand what this gives us. This means I can hack my prefs
such that if I have a single account that gets mail from two addresses such as:

scott@foo.org
and
mscott@mozilla.org

I can now add an identity for each one. Now in the mail compose window, the drop
down list will automatically show all the identies for all accounts. So I will
now have an option for selecting both of these identites instead of just the one
identity I normally associate with the account?

That is very cool!

And as you said the next step would be to get Bug #18906 going next so the user
does not have to manually pick the identity.

I'll try putting this in the next tbird nightly so more folks can try it out.

If someone writes together a nice blurb summarizing all of this I'll make sure
it shows up on the help site (http://texturizer.net/thunderbird/)
FYI: I just posted a test win32 thunderbird build with this fix and a fix i made
up for Bug #18906 which scans through the email addresses of the message being
replied/forwarded and if it finds an email address that matches one of your
multiple identities for the account, it makes the compose window use that
identity by default.

You can find this build here:

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2003-12-16-trunk/
yep, that's exactly what this bug gets you.. as soon as the tree opens for 1.7,
I'll land this.
Blocks: 228788
Alec, I hope you don't mind, I just took the liberty of checking this in along
with my fix for Bug #18906. 

Initial feedback from my test build of thunderbird with this patch has been great!
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: