This problem occurs on both Thunderbird 1.0.7 release build and Thunderbird/nightly/latest-trunk "version 1.6a1 (20051116)". If address book name doesn't contain ASCII alpha-numeric character, address book is lost when new address book is created due to overlay of ldap_2.servers.user_directory_N.xxxx in prefs.js. This happnes always if address book name consists of Japanese Character(double byte characters in Shift_JIS etc.) only. If address book name contains ASCII alpha-numeric, ldap_2.servers.<ASCII alpha-numeric portion only>.xxxx is used, and no problem because ldap_2.servers.<ASCII alpha-numeric portion only>_<Suffix>.xxxx is used if dulicate <ASCII alpha-numeric portion only>. See Bug 218839 Comment #7 for latest usage of ldap_2.servers.xxxx.yyyy. Changes described in Bug 218839 Comment #7 looks to be already applied to Tb 1.0.7 in addition to trunk nightlies. I think this is "Core" issue, although I didn't complete test with Seamonky trunk. [Problem re-creation procedure] (1)Start Thunderbird (2)Create new address book named "+". (3)Shutdown Thunderbird prefs.js entries > user_pref("ldap_2.servers.user_directory_1.description", "+"); > user_pref("ldap_2.servers.user_directory_1.dirType", 2); > user_pref("ldap_2.servers.user_directory_1.filename", "abook-1.mab"); > user_pref("ldap_2.servers.user_directory_1.isOffline", false); > user_pref("ldap_2.servers.user_directory_1.protocolVersion", "2"); (3)Restart Thunderbird (4)Create new address book named "-". ==> Both "+" and "-" is displayed. (5)Shutdown Thunderbird ==> ldap_2.servers.user_directory_1.xxxx is overlayed. prefs.js entries > user_pref("ldap_2.servers.user_directory_1.description", "-"); > user_pref("ldap_2.servers.user_directory_1.dirType", 2); > user_pref("ldap_2.servers.user_directory_1.filename", "abook-2.mab"); > user_pref("ldap_2.servers.user_directory_1.isOffline", false); > user_pref("ldap_2.servers.user_directory_1.protocolVersion", "2"); (6)Restart Thunderbird ==> Only "-" is displayed. ==> address book created at step (2) ("+") is lost. [Workaround] Create address book with ASCII alpha-numeric only, then rename thru property of the address book.

Note: whilst this may look like dataloss to most users, you can still get your original address book back by creating a new a then picking up the older abook-x.mab, i.e. in the case above it would still exist as abook-1.mab.

Mscott or David, how about using "X" of "abook-X.mab" as "N" of "user_directory_N"?

This problem also occurs on Seamonkey 2005111809-trunk/Win-2K. Changing to Product=Core.

A Japanese user has reported "This bug occurs on Thunderbird 1.0.6 release build" to Bugzilla in Japan. When changes of Bug 218839 Comment #7 was introduced to 1.0.x branch?

The problem is here: http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsDirPrefs.cpp#2439 in that function we're incrementing dir_UserId but then not saving it back to prefs (as ldap_2.user_id). Ideally, I think we should be removing dir_UserId completely based on other comments in that file, but we'd need to check the implications of doing that.

Cc-ing to David, because your help is needed. Logic in 1560 char *DIR_CreateServerPrefName is as follows. 1569 if (name) 1570 leafName = nsCRT::strdup(name); 1571 else 1572 leafName = dir_ConvertDescriptionToPrefName (server); 1573 if (leafName) 1574 { (snip, prefName is defined in this block) 1605 } /* if leafName */ 1607 if (!prefName) /* last resort if we still don't have a pref name is to use user_directory string */ 1608 return PR_smprintf(PREF_LDAP_SERVER_TREE_NAME".user_directory_%d", ++dir_UserId); 1609 else 1610 return prefName; When leafName is returned dir_ConvertDescriptionToPrefName(ASCII AlphaNumeric is included in name), duplication check is perfectly done by line 1573 thru 1605. But if no leafName is returned("\0" is retuned when this bug's case), line 1607 thru 1608 is executed. In this case, before changes described in Bug 218839 Comment #7, I guess that %d was the last entry number then uniequness was always kept. However, last entry number is not kept in prefs.js any more after the changes, and %d always starts from 1 when restart of Thunderbird, then overaly of prefs.js entry always occurs if no leafName is returned("\0" is returned). I think next logic between line 1572 and 1573 can be a workaround. if (leafName=="" || leafName=="\0") { leafName = PR_smprintf(PREF_LDAP_SERVER_TREE_NAME".user_directory_%d", ++dir_UserId); } (But it becomes user_directory_NN_XX except first one... Will it be successfully handled?) And next(%d is not used) seems to be sufficient since suffix of "_X" is increased until unieque name is obtained. if (leafName=="" || leafName=="\0") { leafName = PR_smprintf(PREF_LDAP_SERVER_TREE_NAME".user_directory", ++dir_UserId); } (But it bocomes "user_directory", no suffix, when first one... Will it be successfully handled?) David, is it right? Another concern is fall back. Can previous version of Thunderbird handle entry of "user_directory" or entry of user_directory_NN_XX"?

Correction of previous comment. leafName should not include PREF_LDAP_SERVER_TREE_NAME part. Should be ; leafName = PR_smprintf("user_directory_%d",++dir_UserId) ; or ++dir_UserId ; leafName = "user_directory" ;

Comment on attachment 209452 [details] [diff] [review] Based on comment #6 idea with some modifications Mark has been doing stuff with dirPrefs recently, and is much more up to date on it than I am.

Comment on attachment 209582 [details] [diff] [review] Patch rv 1.1: adding more safety check This seems to work alright, just a couple of nits: + if (!leafName || !*leafName) + { + // we need to handle this in case the description has no alphanumeric chars + // it's very common for cjk users + leafName = nsCRT::strdup("_nonascii"); + } Please add one blank line either side of this statement. Also, we normally use 2 space indentation now, so please change the section above to 2 spaces indentation and the additional section to 4 spaces indentation. I'm not entirely sure the second section is necessary, but I don't see much harm in adding it. r=me with the nits fixed.

> Also, we normally use 2 space indentation now, so please change the section > above to 2 spaces indentation and the additional section to 4 spaces > indentation. But some other part of this function seems to assume 4 spaces tab. So I've changed the whole function to 2 spaces indentation.

checked-in.

Comment on attachment 209860 [details] [diff] [review] with the nits fixed This is critical issue and very unpopularity bug in Japan. Let's go to 1.8.0.x and 1.8.1.

(In reply to comment #15) > (From update of attachment 209860 [details] [diff] [review] [edit]) > This is critical issue and very unpopularity bug in Japan. Let's go to 1.8.0.x > and 1.8.1. > Have you checked that the patch will apply to 1.8.x branch? There's been significant changes to some of the rest of nsDirPrefs since we branched.

Comment on attachment 209860 [details] [diff] [review] with the nits fixed Oops. This cannot apply to 1.8 branch. But previous patch can apply to 1.8 branch. I'll attach new patch for 1.8 branch.

Comment on attachment 210227 [details] [diff] [review] Patch for 1.8 branch thx, let me know if you want me to check this into the 1.8.1 branch.

checked-in to 1.8 branch.

Comment on attachment 210227 [details] [diff] [review] Patch for 1.8 branch we're not goint to take this for the security branch. This issue was also present in 1.0.x and has already been fixed for Thunderbid 2.

(In reply to comment #22) > we're not goint to take this for the security branch. This issue was also > present in 1.0.x and has already been fixed for Thunderbid 2. As I said in Bug 218839 Comment #12, this bug was found to be regression over 0.9, regression which should have been resolved before shipment of 1.0.0. So "This issue was also present in 1.0.x" sounds very funny for me as reason of denial. Firefox team seems to have changed(improved) release rule of 1.5.x from one for 1.0.x. (Not security fix only. If big issue, and if possible, resolve it by next minor release. See http://wiki.mozilla.org/Global:1.9_Trunk_1.8_Branch_Plan) Mscott, Thunderbird team won't change(improve) rule for 1.5.x from one for 1.0.x?

Needless to say, I should have found this bug when TB 1.0 RC 1 is released, but I couldn't find until 2005/11/16, then solving of this bug before shipment of TB 1.0.0 was impossible.

Seems like a reasonable stability branch fix that got missed, plussing for reconsideration

Comment on attachment 210227 [details] [diff] [review] Patch for 1.8 branch Re-check this. Daniel agreed to fix this bug on 1.8.0.2.

Your right, we should fix this for 1.8.0.2. Masayuki, can you find some folks to verify the fix on the 1.8.1 branch before we land this on 1.8.0.2? Thanks!

(In reply to comment #27) > Masayuki, can you find some folks to verify the fix on the 1.8.1 branch before > we land this on 1.8.0.2? Verification itself is possible and very easy with folder name of "-" and "+". Didn't you read comment #0?

Comment on attachment 210227 [details] [diff] [review] Patch for 1.8 branch approved for 1.8.0 branch, a=dveditz for drivers

-> v. (trunk and 1.8 branch)