Requirements General requirements o pre-configurate and lock server preferences before deploying them. o the ability to designate which account/server is the default for email or news separately. o when a preference is locked, the associated UI in the client should be disabled/greyed out. o end users should not be able to modify/delete pre-configure settings o the solution should work with both new profiles and existing profiles which might have server settings already. o there should be preference of whether users are allowed to add new mail/news account. o how do we handle pre-existing server settings in old profiles - leave them alone (end users still can use them) but use new server setting as the defaults. - when locked, users won't be allowed to switch the default server. o users will get prompted to enter user information (account name, identity, etc.) if such information is not available in the user profile.

Ninoschka, I should be default QA contact for this bug since this is an customization feature. I may need help testing overall account manager functionality for regressions. Feel free to reassign it back to yourself if you feel you should own this.

Here is the file that contains preconfigured prefs from both all.js and mn_prefs.txt (a file which contains other preconfigured prefs which are to be locked).

I have requested Rodney & Ninoschka to run their test suites with the optimized build I have provided them. Also, I have requested Srilatha and Seth to take a look at the patch and provide any feedback.

looks good. some minor comments / questions: 1) Please add comments to your code. If nothing else, just indicating that these changes add support for pre-configure email/news accounts. The account manager and account wizard code (and mozilla/mailnews in general) is getting pretty complex. The more comments we have, the better. 2) + catch (ex) { + } and + catch (ex) { } Do you expect to get into these catch() blocks, or would getting there be unexpected? If unexpected: catch (ex) { dump("foobar failed: " + ex + "\n"); } If expected: catch (ex) { // we expect to get here in these scenarios... } 3) + var smtpRequiresPrefStr = "mail.identity." + identity.key + ".smtpRequiresUsername"; + smtpRequiresUsername = gPrefs.getBoolPref(smtpRequiresPrefStr); Can we use nsIMsgIdentity's getBoolAttribute(in string name); smtpRequiresUsername = identity.getBoolAttribute(smtpRequiresUsername); 4) + // Time to check if there are any pre-configured accounts and if we need + // add them to the list of existing set of accounts Is there a tab / whitespace issue on that comment? 5) + PRBool addedPreConfigAccounts = false; + rv = m_prefs->GetBoolPref("mail.preconfigaccounts.added", &addedPreConfigAccounts); + + if (NS_SUCCEEDED(rv) && !addedPreConfigAccounts) { + nsXPIDLCString appendAccountList; + rv = m_prefs->CopyCharPref("mail.accountmanager.appendaccounts", + getter_Copies(appendAccountList)); + + if (NS_SUCCEEDED(rv) && appendAccountList.get()) { + if (accountList.get()) { + accountList += ","; + accountList += appendAccountList; + } + else { + accountList = appendAccountList; + } + rv = m_prefs->SetBoolPref("mail.preconfigaccounts.added", PR_TRUE); + } + } } a) What happens if the appendedAccountList contains accounts already in account list? Or, is that not possible since this code only gets called when there are now accounts? b) In the code, how soon after we append this account and set this pref are we flushing prefs to disk? Or does account manager code already flush to disk at the appropriate times? 6) + // Time to check if there are any pre-configured smtp servers and if we need + // add them to the list of existing set of accounts + PRBool addedPreConfigSmtpAccounts = false; + rv = prefs->GetBoolPref("mail.preconfigsmtpservers.added", &addedPreConfigSmtpAccounts); + + if (NS_SUCCEEDED(rv) && !addedPreConfigSmtpAccounts) { + nsXPIDLCString appendServerList; + rv = prefs->CopyCharPref("mail.smtpservers.appendsmtpservers", + getter_Copies(appendServerList)); + + if (NS_SUCCEEDED(rv) && appendServerList.get()) { + if (serverList.get()) { + serverList += ","; + serverList += appendServerList; + } + else { + serverList = appendServerList; + } + rv = prefs->SetBoolPref("mail.preconfigsmtpservers.added", PR_TRUE); + } + } same as 5.... a) What happens if the appendServerList contains accounts already in serverList? Or, is that not possible since this code only gets called when there are now accounts? b) In the code, how soon after we append this server and set this pref are we flushing prefs to disk? Or does compose service code already flush to disk at the appropriate times?

Bhuvan, I took a first look at your optimized build and I was seeing two issues. 1) During the first launch of the optimized Mail/News build with no Mozilla Profiles or 4.x Migration, the wizard screen to prefill my identity information appeared as expected. After prefilling my name and e-mail address and completing the account wizard I was able to use the mail account with no problems. But after I quit the browser and relaunched the mail and newsgroups application, the Account Wizard kicked off once again and asked me to pre-fill my identity information again. 2) If I fill out the first Account Wizard Identity screen traverse to the "Next" screen, click the "Back" button, it takes me all the way back to the "New account setup" screen and asks me to to "Select the type of account you would like to set up: Email Account, Newsgroup account" Since you preconfigured the mailserver information in the all.js file we should never see this screen correct? If I traverse through the next set of screens I am able to specify server information (which I didn't see initially when I ran the wizard the first time) which in turn allows the end user to specify any mail server not pre configured by the IT admin.

Seth, Thanks for your comments. * Comments 1, 2, 4 : I will add comments there and remove any unwanted whitespace chars. * Comment 3 : Yeah, I can use that attribute to do the same. OK, I will change accordingly. * Comment 5, 6 : These just are effective not for just no accounts case. They are to be executed for no accounts/existing accounts/migrated account cases. As far as AccountManager accounts concenred, I will add a check to see if that account already exists and in which case we can prevent adding the samee string twice. As far smtp servers concerned, there can multiple with the same server name, given the way it is right now. At present, all these prefs seemed to be flushed to the prefs file at the appropriate times. thanks Bhuvan

I just spoke to Rodney on the phone to clarify the issues he has faced. Anyway, here are the comments related to those issues. Rodney, Thanks for your comments. Comment 1 : This is happening because in the sample file I have provided, there are both mail and news pre-configured accounts. So, both are invalid accounts. Launching mailnews application looks for any existing invalid accounts and triggers AccountWizard to complete that account again. So, the wizard you have seen second time is to complete the setup of news account. If you just pre-configure mail or news accounts, you will not see the wizard on the second launch. We can enhance it by indicating the account we are trying to setup. Comment 2 : This is a known bug. I think this is the right time to fix that one also. Thanks again, Bhuvan

what are the chances this will be resolved by the end of the week? pls update ETA in the Status Whiteboard. thanks!

I have already given Ninoschka an updated version of optimized so that she can test further. I will be posting an updated patch (with Seth's comments incorporated) soon. Setting ETA to the end of this week (6/15).

New patch addressing comments & suggestions from Seth.

This file has few more new prefs to be part of global prefs file and also some new prefs to be locked in order to achieve locking without introducing new prefs.

fix tabs here + // Time to check if there are any pre-configured accounts and if we need + // add them to the list of existing set of accounts + PRBool addedPreConfigAccounts = false; you can use PR_Free here: + PR_FREEIF(preConfigAccountsStr); PR_FALSE + PRBool addedPreConfigSmtpAccounts = false; you can use PR_Free here: + PR_FREEIF(preConfigSmtpServersStr); "not alredy there in" "already" I'll look some more...

Thanks David. Incorporated the above sugegstions in my local tree.

+ PRBool addedPreConfigAccounts = false; + rv = m_prefs->GetBoolPref("mail.preconfigaccounts.added", &addedPreConfigAccounts); + + // If there are pre-configured accounts, add them to the existing account list + if (NS_SUCCEEDED(rv) && !addedPreConfigAccounts) { Once this pref is set to TRUE, you won't be able to add any pre-configured accounts. This means that for a profile, the preconfigured the accounts can be added only once unless somehow we reset this pref to false. probably the same goes for the smtp servers too "mail.preconfigsmtpservers.added"

Working on adding versioning system so that we can avoid that problem..patch coming up soon. Thanks.

Added versioning system where ISPs can increase the default version pref if tey ever have to add additional pre-configured accounts. The version number stored in user's prefs.js will always be compared with the default version and the process of appending new accounts/servers will happen based on that.

Initially "mail.append_preconfig_smtpservers.version" does not exist in the prefs.js file. So the initial value is 0 and after we add the preconfigured accounts it is set to 1. The value of this pref is 1 in the mailnews.js. + // Update the account list if needed + if ((appendAccountsCurrentVersion <= appendAccountsDefaultVersion)) { + That means, this if condition will be true twice for the same preconfigured accounts. You can fix this by changing "<=" to "<". Same goes for the SMTP servers too. other than this one issue, you have an r=srilatha

I have just initialized those integers (appendAccountsCurrentVersion, appendAccountsDefaultVersion) to ZERO. But the first time around, the value current Version gets is the default one since there is no such pref in the user's prefs.js. Then it gets updated to 2 after adding the accounts/servers. So, next time around the current version is 2 and the default one (in mailnews.js) is 1. So, we will skip the process. When the vendor wants to add more pre-configured accounts/servers, he/she will change the default version (in mailnews.js) to 2 and add all required prefs. Then (since versions are equal, time to upgrade), we will do the upgrade. So, the current set up will work as expected to effect the upgrades as needed. Let me know if I need to address any other issues. Thanks for the review.

Comment on attachment 88101 [details] [diff] [review] patch, v1.2 r=srilatha

+ // Get the list of smtp servers and create keyed server list + if (NS_SUCCEEDED(rv) || appendServerList.get()) { there are a couple instances of this. Do you mean to be checking if appendServerList.Length() > 0 ? And the OR check (||) is a bit odd - how could appendServerList have any data if the call failed?

I have used .get() to check if the string has any content. I can use .Length() > 0. I will incorporate that one. In if (NS_SUCCEEDED(rv) || appendServerList.get()), NS_SUCCEEDED(rv) is used to see if the call to get mail.smtpservers is succeeded and appendServerList.get() to see if we have any pre-configured servers. In either case, we want to build smtpserver list. I can introduce 2 booleans (hasSmtpServers = serverList.Length > 0 and one for appendserverlist) and then check if either them are true, if that will make it more clear. Let me know. Thanks.

+ nsXPIDLCString appendServerList; + prefs->CopyCharPref(PREF_MAIL_SMTPSERVERS_APPEND_SERVERS, + getter_Copies(appendServerList)); + + // Get the list of smtp servers and create keyed server list + if (NS_SUCCEEDED(rv) || appendServerList.get()) { In this code, appendServerList will never have a length > 0 if CopyCharPref call above failed, right? Because appendServerList starts off empty, and then we call CopyCharPref to fill it in. So, I'm claiming that this should just be NS_SUCCEEDED(rv) - does that make sense or am I missing something? If I'm missing something, you could add a comment that explains it...

Per AIM conversation with David, used .Length() > 0 and added more comments where needed.

Comment on attachment 88243 [details] [diff] [review] patch, v1.3 c/f r=srilatha

Comment on attachment 88243 [details] [diff] [review] patch, v1.3 sr=bienvenu

Fixed on the trunk. Marking Fixed. Thanks for all the reviews and suggestions. Adding required keywords.

rvelasco - can you pls verify this on the trunk, then replace fixed1.0.1 with verified1.0.1? thanks!

I think Jaime meant to say to change this bug to verified once it's been verified on the trunk. When it gets landed on the branch, we'll use the 1.0.1 keywords.

I will request Ninoschka to run through the AccountManager test suite in detail.

Trunk build 2002-06-19: WinMe The following tests behaved as expected: 1. 1st Profile - Added an IMAP, POP, AOL, WebMail and News account - Edited the Server Settings (i.e. biff to 1 minute, Empty Trash on Exit) - Edited Copies & Folders so that the Sent, Drafts and Templates pointed to a non-default folder. Then performed a Save As Draft, Save As Template and sent a message and all messages appeared in the appropriate folders. - Sent and Received messages with all scenarios - Set the WebMail account as the default - Removed the WebMail account - Again in all cases I was able to send/receive messages 2. 2nd Profile - Activated a WebMail account, then added an IMAP, POP and AOL account - Edited the Server Settings (i.e. biff to 1 minute, Empty Trash on Exit) - Edited Copies & Folders so that the Sent, Drafts and Templates pointed to a non-default folder. Then performed a Save As Draft, Save As Template and sent a message and all messages appeared in the appropriate folders. - Sent and Received messages with all scenarios - Set the AOL account as the default - Removed all accounts except for the WebMail account - Then added an IMAP account - In all cases I was able to send/receive messages.

Account pre-configuration works as desinged using todays trunk builds on all platforms. Since bug 144563 is somewhat related to this bug I tested it two ways... 1. Preconfigured account with no locking as defined in bug 144563: Success 2. Preconfigured account with locking as defined in bug 144563: Success Tested this on todays (2002061908) trunk builds. Mac OS X.1.3 2002061908 Mac OS 9.2.2 2002061908 Linux 2.4.7-10 2002061908 Windows NT 4.0 2002061908 Marking this bug verified on trunk.

adt1.0.1 (on ADT's behalf) approval for checkin to the 1.0 branch, pending drivers approval. pls check this in asap, then add the "fixed1.0.1" keyword.

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.

over to jimmyu for verification on the branch once it lands.

Fix checked in on the branch. Adding fixed1.0.1 keyword.

jimmy - pls verify this fixed on the 1.0 branch.

Verified on 1.0 branch Mac OS9 20020622 Linux 20020622 Windows 20020622 1.0 branch builds for Mac OSX are not ready. Once it's available will verify then mark this bug accordingly.

Is this feature documented somewhere? Where do I have to put the pre-config file so it gets read during install?

Marking verified1.0.1 MacOSX 20020624