62433 - Multiple mail accounts given the same name when migrated

Status: VERIFIED FIXED

(MailNews Core :: Profile Migration, defect, P2)

Description

[Christos Cheretakis]

Installed linux/2000120808, after removing .mozilla.
Mozilla migrated old mail accounts from ns4.76. There where two IMAP + local.
Although the IMAP accounts where migrated correctly, they're migrated under the
same name, namely that of the first account.

Comment 1

[Keyser Sose]

Reporter can you write out in step by step detai what you did?

Comment 2

[laurel]

Are you saying you got duplicate profile names listed in the profile manager
dialog or that you got extra directories on disk for ImapMail + Mail?  Because
if it's the latter, we have existing bugs about the extra directories being
created.  ( bug 34516 and bug 50567 )

Comment 3

[Christos Cheretakis]

I did not check for directories, so I do not know. I currently am away of my 
primary machine so I cannot test or reproduce it and tell you. All I was saying 
is that, although in my ns configuration I have two profiles, one for 
[e-mail], my primary e-mail account, and one for 
[e-mail], which is my account at work, the migration 
wizard created two accounts in mozilla, where both names are derived from 
[e-mail] (can't check them now), but their parameters are correct. Ie, 
first one has server set to mail.otenet.gr, second to mail.knowledge.gr. I'll 
check for the directories as soon as I get back.

Comment 4

[(not reading, please use [e-mail] instead)]

migration bugs to racham

Comment 5

[Keyser Sose]

Christos can you update us with any new information?

Comment 6

[Christos Cheretakis]

2001012606 changed behaviour to the worse!
Here is what I did:
- move .mozilla to .oldmozilla
- run mozilla
- it asks whether it should migrate existing ns4 profiles
- when its done, there are two mail account profiles, with the exact same
information

previous build I checked with, migrated information correctly, but only missed
the name.

Comment 7

[Keyser Sose]

Marking NEW as per comments.

Comment 8

[Matthew T (active 1999-2002)]

Reproduced on build 2001052115, Mac OS 9.1; --> All/All. Highly visible bug.

Comment 9

[Ninoschka Baca]

Marking nsbeta1 because this is too confusing.

Buid 2001-05-22-04: WinMe

Steps I used to reproduce the problem:
1. In 4.7 configure for one profile with 2 IMAP accounts (i.e. qatest22, 
qatest33)
2. To migrate the profile I used the -installer flag

Actual Results:
a. * When Mail starts I can see two accounts, both called [e-mail].
b. In Account Settings the settings for:
- the first account is correct. All the panels reference qatest22. 
- * the second account's Identity panel is wrong, ref qatest22
- the second account's Server panel is correct, ref qatest33
- * the second account's Copies and folder panel is wrong, ref qatest22
c. Login to the first account and the login dialog references qatest22, as 
expected.
d. Login to the second account and the login dialog references qatest33 and 
retrives the correct messages.
3. * Create a new message and all the identities reference qatest22

Note: Items with an "*" are specific areas that display problems.

Comment 10

[Ninoschka Baca]

Setting QA Contact to nbaca since this problem appears in Mail and not the 
Profile Manager.

Note: In my previous scenario qatest22 and qatest33 use different mail servers.

Comment 11

[scottputterman]

moving to 0.9.2

Comment 12

[kmurray1115]

This should get fixed, but seems to affect a small percentage overall. I'm
comfortable with it being in 0.9.2, as opposed to 0.9.1. 

Comment 13

[scottputterman]

adding PDT+

Comment 14

[racham]

Attachment: patch to make the migrated accounts to have independent Account Names

Comment 15

[laurel]

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

Comment 16

[laurel]

Is only the account name going to be affected or are we going to change anything
in the identity panel, too?  Although we here know that there was only one
Identity information set in 4.x to migrate, it seems the folks which get
confused by the same account name also get confused by the identity panel info
being the same. 

Comment 17

[racham]

Sorry. I wanted to update this bug further but just got delayed...

The patch I posted is going to effect only the Account Name (each account will
have unique account name) that appears on folder pane, Account Central and the
Account Settings panel's account name.

Actually as you mentioned this is a single identity. It just has 2 imap accounts
4.x. We need to maintain the integrity without doing any data manipulations at
identit level. So, the panels will look the same except for the Account name (&
user & host names). I hope users sophisticated enough to manage 2 imap accounts
in 4.x will understand the unchanged identity information.

Comment 18

[(not reading, please use [e-mail] instead)]

if I used a previous version (without this patch) and I migrated (instead of
using the wizard) the ".name" value for my imap server would not be set.

so my imap server "name" (in the folder pane) would be "[e-mail]".

after this patch, if I start up again, the name would be "sspitzer@nsmail-1". 
it would have changed on our existing users.

note, if the user used the wizard, they'd have set the pretty name and this
wouldn't be a problem.

Is this scenario a problem?  changing the pretty name on users doesn't seem like
a good thing.   if we are ok with that, then this would be the right fix.

otherwise, since we are worrying about migration, we could fix
nsMessengerMigrator.cpp so that it sets the pretty name for imap servers on
migration.  this would make it so that only new users are effected.

postscript:  please, let's not even talk about what happens to users who have
two accounts with the same username and hostname, but different ports.

Comment 19

[racham]

Attachment: alternative patch to take care the problem Seth mentioned. Changes made to MessengerMigrator.

Comment 20

[(not reading, please use [e-mail] instead)]

this is the right idea, but you've made base/src depend on imap.

what you really want is to add this to the nsIMsgIncomingServer interface:

// only used during migration
wstring prettyName generatePrettyNameForMigration();

in the base implementation in nsMsgIncomingServer, you'd return
NS_ERROR_NOT_IMPLEMENTED.

in the nsImapIncomingServer.cpp, you'd override it to return "username@hostname"
[see below]

right now, the only place we'd call GeneratePrettyNameForMigration() would be
where you're calling GetPrettyNameForImapAccount().  but do it AFTER the call to
set the server port.

here's why:  back to my previous comment about "let's not even talk about what
happens to users who have two accounts with the same username and hostname, but
different ports."

you can address that if you make the imap implementation of
GeneratePrettyNameForMigration() do this:

prettyName = username + "@" + hostname;
if (port != default_port) {
  prettyName += ":" + port;
}

keep in mind default_port will change if the server is secure or not.

Comment 21

[racham]

Attachment: updated patch - with generatePrettyNameForMigration() added

Comment 22

[(not reading, please use [e-mail] instead)]

this looks better, but you don't need those parameters to
generatePrettyNameForMigration()

It should be 
+  wstring generatePrettyNameForMigration();

in the implementation [nsImapIncomingServer::GeneratePrettyNameForMigration()],
you should get the port, [GetPort()], figure out if the server is secure
[GetIsSecure()] and get the default port (secure or not) from the protocol info.

there is no reason to pass that in, since you call
GeneratePrettyNameForMigration() after port and isSecure are set.  (you should
add a comment above the call to GeneratePrettyNameForMigration() in
nsMessengerMigrator.cpp that GeneratePrettyNameForMigration() has to be called
after port and isSecure is set. 

Comment 23

[racham]

Attachment: new patch (with nsImapIncomingServer doing the most work)

Comment 24

[(not reading, please use [e-mail] instead)]

two minor comments:

1)

+    if ((serverPort == defaultServerPort) || 
+        ((serverPort == defaultSecureServerPort) && isSecure)) {
+        isItDefaultPort = PR_TRUE;
+    }

that should be 

if (((serverPort == defaultServerPort) && !isSecure) ||
    ((serverPort == defaultSecureServerPort) && isSecure)))

2)  

I think you can remove a string copy in nsImapIncomingServer::GetFormattedName()

I think you can do just this:

+// Take the pretty name and return a formatted account name
+nsresult
+nsImapIncomingServer::GetFormattedName(const PRUnichar *prettyName, PRUnichar
**retval)
+{
+        const PRUnichar *formatStrings[] =
+        {
+            prettyName,
+        };
+        rv = m_stringBundle->FormatStringFromID(IMAP_DEFAULT_ACCOUNT_NAME,
+                                    formatStrings, 1,
+                                    retval);
+        NS_ENSURE_SUCCESS(rv, rv);
+    }
+    return rv;
 }

Comment 25

[racham]

Attachment: Diffs to nsImapIncomingServer.cpp with changes suggested by Seth

Comment 26

[racham]

got rid of now unused
+        nsXPIDLString formattedName;

also.

Comment 27

[(not reading, please use [e-mail] instead)]

sr=sspitzer on the last patch.

Comment 28

[racham]

Navin is currently reviewing the patch.

Comment 29

[Navin Gupta]

r=naving; although I am not sure if port stuff is really required. 

Comment 30

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 31

[racham]

Marking Fixed.

Comment 32

[Ninoschka Baca]

Build 2001-06-22-11: NT4 (Fenella's system)
Build 2001-06-22-04: Mac 9.04
Build 2001-05-22-06: Linux RH 6.2
Verified Fixed. The accounts appear unique in the folder pane and Account 
Settings.