Per mail triage meeting, the current listing of the imported Outlook mail and folders is confusing (see bug 70540). Since we can't fix bug (70644) which would have avoided the confusion, we need identify the imported mail better. Overview: When importing "Mail", the name of the account assumes the name of the original email application (i.e. Outlook Express) Steps to reproduce: 1. Create a new profile 2. When Mail starts cancel the Account Wizard 3. Select File|Import 4. Select the Mail radio button, Next 5. Select Outlook Express, Next Actual Results: The name of the account that appears in the folder pane is "Outlook Express". Expected Results: The name should be "Imported Mail from Outlook Express" So the user does not think the Outlook Express account was imported with settings. All we imported were the folders with messages.

adding nsbeta1 because fixing this bug alleviates the need to fix bugs 70644 and 70540 now.

I don't think this solution is going to fix the confusion for users. First, the name is too long and will be truncated. Second, even though the first part of the name is "Imported Mail...", the confusion is not necessarily the name, but in the fact that we are showing Imported Local messages as an Account, which it isn't. It will look like an account, but won't behave, as users will expect, like the other accounts. As I mentioned in 70644: I would expect that these messages would be grouped UNDER our Local Folders. Not as a separate account. An account wasn't created. Local messages from a different application were pulled in. For example: Local Folders Unsent Messages Drafts Templates Sent Trash Outlook Express Mail (subfolders within this folder as nec)

Is this really a nsbeta1+, P2? If yes, then we need to try and targeted to a milestone M1.0 or earlier to make the beta.

I'm ok with what jglick suggested on http://bugzilla.mozilla.org/show_bug.cgi?id=86991#c2 minor questions: 1) what if I import multiple times, or if the folder already exists? Do we want to append a unique number? Local Folders Trash Drafts Outlook Express Mail Outlook Express Mail 2 Outlook Express Mail 3 or perhaps: Local Folders Trash Drafts Outlook Express Mail Outlook Express Mail (date and time) Outlook Express Mail (date and time) The good thing about this is users could rename the folder pretty easily. 2) to clarify, we will not be treating the imported folders as special folders when we import mail. The will just be generic folders. please confirm (and clarify) and update the spec, so I know it's official.

>Outlook Express Mail >Outlook Express Mail 2 >Outlook Express Mail 3 Appending a number is fine (and shorter!). I will try and update the spec soon.

we'd want this same behavior for imported 4.x mail, right? if so, srilatha might be the one to take this, as she's working on that, and implementing this will make her life easier. over to srilatha.

Yeah, we'd want this for imported 4.x code since we won't be associating it with an account.

Comment on attachment 70758 [details] [diff] [review] patch v1 The folder creation part looks ok to me, one thing though there is an accessor GetRootMsgFolder on server.Use that directly to get localRootFolder

Now using GetRootMsgFolder.

Comment on attachment 70778 [details] [diff] [review] patch v2 r=naving

+ if (m_pName) { + name.AssignWithConversion(m_pName); + name.Append(" mail"); + } + else + name.Assign("Imported Mail"); do those strings show up in the UI? if so, we have to move them to a string bundle. looks like we'll need two strings, one "%S mail" (which we'll need to use the formatter) and one for "Imported Mail".

updating summary to reflect what srilatha is going for. srilatha, this change will affect import of all mail types: OE, Outlook, Eudora and eventually 4.x local mail, right? please make sure to test those, and to test mac. (we have at least import of eudora on mac.) note to jglick: are we worried about the discoverability of imported mail? perhaps in the import wizard, after import, we should tell the user where they can find there imported mail?

Tested this patch on windows with OE and outlook. will test with Eudora on Mac and windows

>we'd want this same behavior for imported 4.x mail, right? Importing Messages only, yes. When importing a whole account, it should be at account level. >to clarify, we will not be treating the imported folders as special folders >when we import mail. The will just be generic folders. Yup. >note to jglick: are we worried about the discoverability of imported mail? >perhaps in the import wizard, after import, we should tell the user where they >can find there imported mail? what about expanding the Local Mail twistie and focusing the imported folder in 3 pane on completion of import?

>what about expanding the Local Mail twistie and focusing the imported folder in >3 pane on completion of import? If the user is importing Mail from any other window other than the Mail window this is not possible. Having a message with the location of the imported mail in the import wizard would be good.

Showing text on the last screen of the Import Wizard is fine too. :-)

Tested with Eudora on Mac and windows. I will file a separate bug for the message in the Import wizard

this code isn't i18n safe. looking at it, if localizers set to anything other than US-ASCII, it won't work. +DefaultFolderName=Imported Mail +ModuleFolderName=%S Mail cc nhotta and bienvenu for comments. two minor issues, and the on big i18n issue: 1) don't add this: +static NS_DEFINE_CID(kStringBundleServiceCID, NS_STRINGBUNDLESERVICE_CID); instead use this: NS_STRINGBUNDLE_CONTRACTID 2) instead of: + if (NS_FAILED(rv) || !bundleService) return PR_FALSE; + if (NS_FAILED(rv)) return PR_FALSE; do this: if (NS_FAILED(rv) || !bundleService) return PR_FALSE; if (NS_FAILED(rv)) return PR_FALSE; the reason for this convention (inspired by waterson) is so that you can set a break point on the return. 3) + nsCString name; + if (m_pName) { + const PRUnichar *moduleName[] = { m_pName }; + rv = bundle->FormatStringFromName(NS_LITERAL_STRING("ModuleFolderName").get(), + moduleName, 1, + getter_Copies(folderName)); + } + else { + rv = bundle->GetStringFromName(NS_LITERAL_STRING("DefaultFolderName").get(), + getter_Copies(folderName)); + } + if (NS_FAILED(rv)) { + IMPORT_LOG0( "*** Failed to get Folder Name!\n"); + return PR_FALSE; + } + name.AssignWithConversion(folderName); This conversion will fail in the property is non-US-ASCII, right? later, I see you go back from char * to PRUnichar * to call createSubfolder(). The problem is you're building on non-i18n friendly code written along time ago. These two methods of nsIMsgFolder boolean containsChildNamed(in string name); string generateUniqueSubfolderName(in string prefix, in nsIMsgFolder otherFolder); and this method of nsIFolder nsISupports GetChildNamed(in string name); need to be fixed to use wstring. Luckily, I think they are only used by the import code.

I should add that the existing import code doesn't look i18n friendly either.

1) and 2) I will fix those in my next patch I agree with Seth on 3). If we can change the string to wstring the i18N issue will be solved.

In this patch I changed the two methods of nsIMsgFolder boolean containsChildNamed(in string name); string generateUniqueSubfolderName(in string prefix, in nsIMsgFolder otherFolder); to boolean containsChildNamed(in wstring name); string generateUniqueSubfolderName(in wstring prefix, in nsIMsgFolder otherFolder); and the method of nsIFolder nsISupports GetChildNamed(in string name); to nsISupports GetChildNamed(in wstring name);

Looks fine with the wstring changes. One question, where does the folder name come from? Is that always taken from the bundle? If anything coming from the actual file name then we need to convert that to Unicode.

In ::CreateFolder folderName either comes from the string bundle or an int appended to the sting bundle. In ImportMailThread lastName is being passed to these methods This is how lastName is assigned a value. This is existing code ------- pName = nsnull; box->GetDisplayName( &pName); if (pName) { lastName = pName; //pName is a wstring nsCRT::free( pName); } else lastName.Assign(NS_LITERAL_STRING("Unknown!")); ---- NS_IMETHOD GetDisplayName( PRUnichar **pName) { *pName = ToNewUnicode(m_displayName); return( NS_OK);} ---- I think we dont need to convert any of these.

>I think we dont need to convert any of these. You are right, they are already Unicode.

looks great. sr=sspitzer three minor issuse. (two of these these aren't in your code, but it's in the code you are touching. can you see if we can fix these while you are in there?) 1) +nsISupports GetChildNamed(in wstring name); use interCaps, so this should be: +nsISupports getChildNamed(in wstring name); luckily, there don't appear to be any js calls to this, so you only have to change the idl. 2) + PRUnichar *pSubName = nsnull; + curProxy->GenerateUniqueSubfolderName( lastName.get(), nsnull, &pSubName); if (pSubName) { - strName.Assign(pSubName); + lastName.Assign(pSubName); nsMemory::Free(pSubName); } use an nsXPIDLString to simplify this code. nsXPIDLCString subName curProxy->GenerateUniqueSubfolderName( lastName.get(), nsnull, getter_Copies(subName); if (!subName.IsEmpty()) { strName.Assign(pSubName); lastName.Assign(pSubName); } 3) + if (exists) { + PRUnichar * pName = nsnull; + localRootFolder->GenerateUniqueSubfolderName(folderName.get(), nsnull, &pName); + if (pName) { + folderName.Assign(pName); + nsMemory::Free(pName); + } + else { again, use an nsXPIDLString to simplify this code.

Made changes based on Seth's comments.

Comment on attachment 73989 [details] [diff] [review] using xpidlstring sr=sspitzer

Comment on attachment 73989 [details] [diff] [review] using xpidlstring a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Fix checked in.

Trunk build 2002-03-22: WinMe, Mac 9.1 Verified Fixed. Checked Eudora, Outlook, Outlook Express. Importing 4.x Folders is being checked in a separate bug.