from bug http://bugzilla.mozilla.org/show_bug.cgi?id=106718 just to avoid bugs like http://bugzilla.mozilla.org/show_bug.cgi?id=106718 it could be nice if the pref "mail.smtpservers" was striped for dupes. So that: user_pref("mail.smtpservers", "smtp2,smtp2,smtp1,smtp2"); became: user_pref("mail.smtpservers", "smtp1,smtp2");

I have 3 SMTP servers and 7 accounts and mail.smtpservers constantly gets corrupted ala "smtp2,smtp2,smtp1,smtp2" 20020516

this bug is now so serious that I have to manually reset every second day. I keep getting "unable to send, check your settings". If I strip dupes in the mail.smtpservers it works.

This is not 100% related, but close. I've had my profile for a while. During its lifetime, I've deleted/re-added some of my mail/news accounts. Now, in my mail.smtpservers list, I have accounts that were deleted. For example, it lists smtp2, which I don't have. So, it seems that the right thing to do would be to check the list for dups AND verify it against reality as well.

Is this relatet to Bug 96207, Bug 69630 or Bug 132948? At least the last seems to be a dupe to this, no?

taking over from bhuvan who is on sabbatical.

Obtain the pref in a tempServerList and then parse it for duplicates and then create the serverList. This will get rid of the dupes for existing users of older builds as well as any new situations.

Comment on attachment 100119 [details] [diff] [review] Patch V 1.0 if tmpServerList is empty, i dont think you need to do this serverList = tmpServerList. so you can get rid of that else part. r=srilatha

Comment on attachment 100119 [details] [diff] [review] Patch V 1.0 needs work. 1) + if (tempServerList.Length() > 0) { why aren't you doing this: if (!tempServerList.IsEmpty()) { 2) + char *tempSmtpServersStr = ToNewCString(tempServerList); Curious, why ToNewCString() and not nsCRT::strdup()? 3) + char *tempToken = nsCRT::strtok(tempSmtpServersStr, SERVER_DELIMITER, &tempSmtpServerStr); + while (tempToken) { + if (tempToken && *tempToken) { Why the check for tempToken? It's not needed, it will always be non null. 4) + char * str = PL_strstr(serverList.get(), tempToken); + if (!str) { There is no need for str. you can just do: if (!strstr(serverList.get(), tempToken)) No need for PL_strstr(), either. 5) + PR_Free(tempSmtpServersStr); It's possible, but unlikely, that tempSmtpServerStr will be null. You should probably check the allocation and return NS_ERROR_OUT_OF_MEMORY.

Made the changes as per the previous review.

accepting and targeting for varada...this should make it for 1.2beta.

Comment on attachment 100687 [details] [diff] [review] Patch V1.1 sr=sspitzer while you're in this code, can you make a #define for "mail.smtpservers" and use it from here and saveKeyList()? no need for a new patch, just make that cleanup locally before you check in.

Added check for null serverList before doing strstr as well as added #defines for mail.smtpservers pref.

Comment on attachment 100768 [details] [diff] [review] Patch V1.2 sr=sspitzer

Marking Fixed.

using build 20020930 it doesn't strip dupes. I have: user_pref("mail.smtpservers", "smtp1,smtp1"); and the dupe isn't stripped... is the fix in that build?

verified on winxp. build 20021001 stripes dupes

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