Closed Bug 70396 Opened 24 years ago Closed 22 years ago

Non-ascii name of migrated 4.x address book is corrupted

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, defect, P3)

x86
Windows NT
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.4alpha

People

(Reporter: marina, Assigned: cavin)

References

Details

(Keywords: intl, relnote, Whiteboard: [adt2][correctness])

Attachments

(3 files, 3 obsolete files)

**** observed with 2001-02-27 build **** Steps to reproduce: - create an AB in 4.x with non-ascii name; - migrate it to Nscp6; - open Address Book; //note: all non-ascii chars in AB names are removed (btw: the migrated AB are all empty but this is a core problem and i'll now if there is a bug filed on that)
Add momoi to cc, we had a problem with addressbook migration before RTM, but that was fixed, right?
yes, the problem was with non-ascii data inside the migrated addressbook (similar to this one: non-ascii chars were removed but the address book name itself had no pproblem), now the addressbook name has a problem and there are no contents after migratin, it's empty. assigning myself as QA contact
QA Contact: ji → marina
The one for non-ascii address book CARD (not address book name) at rtm time is bug 57151.
Assignee: nhotta → chuang
Keywords: intl
Reassign to chuang@netscape.com.
nominating for nsbeta1
Keywords: nsbeta1
marking nsbeta1+
Priority: -- → P3
Whiteboard: [nsbeta1+]
Target Milestone: --- → mozilla0.9
moving to mozilla0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
I just wanted to verify that you are saying that all cards inside these Non-ascii address books are lost?
Priority: P3 → P2
not now, all cards insided the addressbook are displaying fine, so there is no data loss. the only problem is the address book name : for japanese name i see " user_directory1.ldif"
Thanks Marina. Then I'm going to move this to 0.9.2.
Priority: P2 → P3
Target Milestone: mozilla0.9.1 → mozilla0.9.2
more info : non-ascii names are corrupted ( stripped off accented chars or showing "user_directory1" for double-byte ) only the first time you run Netscp6 after migration, the second time you open this profile it shows up correctly.
moving to 0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Marking as nsCatFood, because of migration (i.e. see Project Goals).
Keywords: nsCatFood
just confirming my comments of 2001-05-01 14:00 -----: non-ascii AB name shows up without accented chars for Latin-1 and as "user_directory1.ldif" for DB name only the first time you open application after migration. Close-reopen will fix this problem ... though all ldif directories from 4.x are gone... ( netcenter, verisign and so on...)
moving to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Keywords: nsBranch
Component: Internationalization → Address Book
Selected as one of the 3 top intl bugs for eMojo mail
Whiteboard: [nsbeta1+] → [nsbeta1+] [nsBranch+]
Keywords: nsbranch+
keyword magic. Nothing to see here.
Keywords: nsbranch
Whiteboard: [nsbeta1+] [nsBranch+] → [nsbeta1+]
Move to 0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
chuang, is there any update in this bug?
Why are we nominating this one, if there isn't a patch? Propose we minus
why don't we go the other way around and work on the patch..? :-) This one is more cosmetic because the problem goes away wjehn close/reopen AB, though the user doesn't know it and it look really ugly after migration.
I'm working on it. I don't see the garbage-like address book name with Marine's file. What I see is we are using the filename instead of the real display name.
Blocks: 41887
Blocks: 93438
Attached patch Patch for the fix (obsolete) (deleted) — Splinter Review
This patch also include the fix for bug 41887, 93438.
Adding correctness Status Whiteboard, correct/expected behavior does not occur.
Whiteboard: [nsbeta1+] → [nsbeta1+],[correctness]
Are you pursuing the reviews for the patch? It's pretty large. I'm not totally convinced this is stop-ship, but if it were reviewed and fully tested by 9/26 I'd consider it...
The patch for this does look pretty large and we've lost our address book expert so we don't have anyone right now who can guage the risk for this patch. I'd like to nominate nsBranch- for this. We'll get it into the trunk and get plenty of testing time for the next release.
Scott, feel free to change it to nsbranch-. It's too late now. Marina, make sure this goes into the international section in the release notes.
Keywords: relnote
thanks. putting in the next milestone so it'll make the next release.
Keywords: nsbranch+nsbranch-
Target Milestone: mozilla0.9.5 → mozilla0.9.6
reassigning to cavin
Assignee: chuang → cavin
moving to 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Blocks: 104166
Blocks: 107067
Keywords: nsbranch-
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Keywords: nsbeta1+, patch
Whiteboard: [nsbeta1+],[correctness] → ,[correctness]
this bug has a lot of keywords but is still NEW. Cavin, it is really ugly for Intl users, are we planing to fix it in this release?
It's an accepted bug (the nsbeta1+) but there's a very high chance that because it's a P3 that it won't be. Changing the milestone to reflect the fact that it hasn't been targetted yet.
Target Milestone: mozilla0.9.9 → ---
Blocks: 122274
Status: NEW → ASSIGNED
Keywords: nsbeta1+nsbeta1-
No longer blocks: 107067
Scott, i am confused.. Do we plan to fix it in this release, do i re-nominate? Thanks.
No, we don't plan to fix it in this release. This was a bug that we'd like to fix, but we don't have enough time. If you want to renominate, you should change the nsbeta1- to an nsbeta1.
Discussed in mail news bug meeting. Decided to minus this bug.
Keywords: nsbeta1nsbeta1-
Target Milestone: --- → mozilla1.2alpha
Blocks: 157673
move nsbeta1- to nsbeta1 . please consider this as nsbeta1+ for m1.2final
Keywords: nsbeta1-nsbeta1
Summary: Non-ascii name of migrated address book is corrupted → Non-ascii name of migrated 4.x address book is corrupted
Gregg, can you escalate this bug to the mail team? it seems to be a nasty issue. (this is dassi btw, not simon)
This one has been around for quite a while...it seems we have a patch. Why don't we try to get it on the trunk, get some testing on it, and get it for Buffy?
i18n triage team: nsbeta1+/adt2
Keywords: nsbeta1nsbeta1+
Whiteboard: ,[correctness] → [adt2][correctness]
Target Milestone: mozilla1.2alpha → mozilla1.4alpha
Attached patch Proposed patch, v1 (obsolete) (deleted) — Splinter Review
In ParseFile(), try to use 'description' pref value as the dir/addrbook name if it exists, otherwise use the leafname of the addrbook filename. Currently we only do this for persoanl addrbook. The problem with missing cards in the migrated addrbooks is caused by the fact that the na2 files were not copied over to the new profile directory so when we tried to convert/import the 4.x addrbooks no cards were ever migrated. So CopyFilesByExtension() is created to do that during migration and the na2 files are removed once they are migrated.
Attachment #50180 - Attachment is obsolete: true
Attachment #117040 - Flags: superreview?(sspitzer)
> The problem with missing cards in the migrated addrbooks is caused by the fact > that the na2 files were not copied over to the new profile directory > This is a regression from July 4, 2002 for Bug 15424. The original code was: rv = DoTheCopy(oldProfilePath, newProfilePath, PR_FALSE); 'oldProfilePath' points to the root of the 4.x dir so all files were copied (including .na2 files). The change was to replace the above with: // just copy what we need rv = DoTheCopy(oldProfilePath, newProfilePath, COOKIES_FILE_NAME_IN_4x); . . . So we do selective copies now and so we don't copy .na2 files any more.
let's start with the change to nsPrefMigration.cpp / nsPrefMigration.h as that will fix http://bugscape/show_bug.cgi?id=21642 1) for this code: + nsFileSpec fileOrDirName = dir.Spec(); //set first file or dir to a nsFileSpec + fileOrDirNameStr.Assign(fileOrDirName.GetLeafName()); + + if (fileOrDirName.IsDirectory() || !nsCStringEndsWith(fileOrDirNameStr, fileExtension)) + continue; do this instead: + nsFileSpec fileOrDirName = dir.Spec(); //set first file or dir to a nsFileSpec + if (fileOrDirName.IsDirectory()) + continue; + + nsCAutoString fileOrDirNameStr.Assign(fileOrDirName.GetLeafName()); + if (!nsCStringEndsWith(fileOrDirNameStr, fileExtension)) + continue; that saves us from getting the leaf name for directories. 2) for this: + rv = fileOrDirName.CopyToDir(newPath); + NS_ASSERTION(NS_SUCCEEDED(rv),"failed to copy file"); + NS_ENSURE_SUCCESS(rv,rv); just do + rv = fileOrDirName.CopyToDir(newPath); + NS_ENSURE_SUCCESS(rv,rv); you don't need that assertion, as NS_ENSURE_SUCCESS(rv,rv) will assert. (it won't have as useful of an error message, but we will still know where it is asserting) 3) + nsresult rv; + nsFileSpec oldPath; + nsFileSpec newPath; + + rv = oldPathSpec->GetFileSpec(&oldPath); instead: + nsFileSpec oldPath; + nsFileSpec newPath; + nsresult rv = oldPathSpec->GetFileSpec(&oldPath); just a style nit. 4) before you call CopyFilesByExtension(), do this: #include "nsIAbUpgrader.h" ... // if we have the upgrader, copy over the .na2 files // so we can migrate them. nsCOMPtr <nsIAbUpgrader> abUpgrader = do_GetService(NS_AB4xUPGRADER_CONTRACTID); if (abUpgrader) { // Copy the addrbook files rv = CopyFilesByExtension(oldProfilePath, newProfilePath, ADDRBOOK_FILE_EXTENSION_IN_4X); NS_ENSURE_SUCCESS(rv,rv); } can you attach a new patch for nsPrefMigration.cpp / nsPrefMigration.h, so I can r/sr seperate from the i18n issue?
Attached patch Fix for empty addrbook migration (obsolete) (deleted) — Splinter Review
Attachment #117175 - Flags: superreview?(sspitzer)
Comment on attachment 117175 [details] [diff] [review] Fix for empty addrbook migration this looks good, but I forgot that using the nsIAbUpgrader would add a dependency on mailnews addrbook. (remember, you can build mozilla with mailnews disabled, and also install mozilla as browser only.) let me double check with ccarlen and alecf about this.
waiting for feedback from ccarlen, but it is looking like adding the dependency on addrbook isn't the way to go. sorry about suggesting it. can you attach a new patch, that always copies over the .na2 files (like you originally had it?)
Attachment #117175 - Attachment is obsolete: true
Comment on attachment 117236 [details] [diff] [review] Fix for empty addrbook migration w/out abupgrader. sr=sspitzer requesting review from ccarlen.
Attachment #117236 - Flags: superreview+
Attachment #117236 - Flags: review?(ccarlen)
Comment on attachment 117236 [details] [diff] [review] Fix for empty addrbook migration w/out abupgrader. r=ccarlen
Attachment #117236 - Flags: review?(ccarlen) → review+
Comment on attachment 117040 [details] [diff] [review] Proposed patch, v1 just a few small nits on the parts of this patch (nsPrefMigration.cpp/.h are part of another patch.) 1) + if (PL_strcmp(fileName, kPersonalAddressbook) == 0) I know you are just moving that code, but let's do this instead: + if (strcmp(fileName, kPersonalAddressbook) == 0) 2) + // If a name is found then use it, otherwise use the filename + // as last resort. (ie, it's ok to ignore rv here). + if (!dirName.IsEmpty()) why not: + // If a name is found then use it, otherwise use the filename + // as last resort. + if (NS_SUCCEEDED(rv) && !dirName.IsEmpty()) if GetLocalizedUnicharPref() fails, (because there is no .description pref), we don't need to check if dirName is empty. 3) + + // If a name is found then use it, otherwise use the filename + // as last resort. (ie, it's ok to ignore rv here). + if (! dirName.IsEmpty()) + parentDir->CreateDirectoryByURI(dirName, mDbUri, mMigrating); else + { + nsAutoString fileString; + fileString.AssignWithConversion(leafName); parentDir->CreateDirectoryByURI(fileString.get(), mDbUri, mMigrating); + } instead: + // If a name is found then use it, otherwise use the filename + // as last resort. + if (NS_SUCCEEDED(rv) && !dirName.IsEmpty()) + dirName.AssignWithConversion(leafName); + + rv = parentDir->CreateDirectoryByURI(fileString.get(), mDbUri, mMigrating); + NS_ASSERTION(NS_SUCCEEDED(rv), "failed to create directory"); can you attach a new patch, excluding the nsPrefMigration code?
Attachment #117040 - Flags: superreview?(sspitzer) → superreview-
Attachment #117040 - Attachment is obsolete: true
Comment on attachment 117290 [details] [diff] [review] Fix for setting the right non-ascii addrbook names. one fine string suggestion: + nsCAutoString prefName; + prefName = NS_LITERAL_CSTRING("ldap_2.servers.") + nsDependentCString(leafName) + NS_LITERAL_CSTRING(".description"); after that, r/sr=sspitzer don't forget to check in your change to nsMessengerMigrator.cpp, too. nice work, cavin.
Attachment #117290 - Flags: superreview+
> one fine string suggestion: I meant, one final string suggestion. that came out as tooting my own horn. beep beep.
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment on attachment 117523 [details] [diff] [review] Remove .na2 file once it's migrated. r/sr=ssputzer, I think I already reviewed this, right?
Attachment #117523 - Flags: review+
Last fix checked in.
it is fixed, after migration of 4.x the non-ascii AB name shows correctly
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: