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)
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)
(deleted),
patch
|
ccarlen
:
review+
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
sspitzer
:
review+
|
Details | Diff | Splinter Review |
**** 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)
Comment 1•24 years ago
|
||
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.
Comment 4•24 years ago
|
||
Reassign to chuang@netscape.com.
Comment 6•24 years ago
|
||
marking nsbeta1+
Priority: -- → P3
Whiteboard: [nsbeta1+]
Target Milestone: --- → mozilla0.9
Comment 8•24 years ago
|
||
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"
Comment 10•24 years ago
|
||
Thanks Marina. Then I'm going to move this to 0.9.2.
Priority: P2 → P3
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Reporter | ||
Comment 11•24 years ago
|
||
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.
Comment 13•23 years ago
|
||
Marking as nsCatFood, because of migration (i.e. see Project Goals).
Keywords: nsCatFood
Reporter | ||
Comment 14•23 years ago
|
||
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...)
Updated•23 years ago
|
Component: Internationalization → Address Book
Comment 16•23 years ago
|
||
Selected as one of the 3 top intl bugs for eMojo mail
Whiteboard: [nsbeta1+] → [nsbeta1+] [nsBranch+]
Comment 17•23 years ago
|
||
keyword magic. Nothing to see here.
Keywords: nsbranch
Whiteboard: [nsbeta1+] [nsBranch+] → [nsbeta1+]
Comment 19•23 years ago
|
||
chuang, is there any update in this bug?
Comment 20•23 years ago
|
||
Why are we nominating this one, if there isn't a patch? Propose we minus
Reporter | ||
Comment 21•23 years ago
|
||
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.
Comment 22•23 years ago
|
||
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.
Comment 23•23 years ago
|
||
Comment 24•23 years ago
|
||
This patch also include the fix for bug 41887, 93438.
Comment 25•23 years ago
|
||
Adding correctness Status Whiteboard, correct/expected behavior does not occur.
Whiteboard: [nsbeta1+] → [nsbeta1+],[correctness]
Comment 26•23 years ago
|
||
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...
Comment 27•23 years ago
|
||
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.
Comment 28•23 years ago
|
||
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
Comment 29•23 years ago
|
||
thanks. putting in the next milestone so it'll make the next release.
Updated•23 years ago
|
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Updated•23 years ago
|
Reporter | ||
Comment 32•23 years ago
|
||
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?
Comment 33•23 years ago
|
||
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 → ---
Updated•23 years ago
|
Updated•23 years ago
|
Blocks: profile-corrupt
Reporter | ||
Comment 34•23 years ago
|
||
Scott, i am confused.. Do we plan to fix it in this release, do i re-nominate?
Thanks.
Comment 35•23 years ago
|
||
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.
Updated•22 years ago
|
Comment 36•22 years ago
|
||
Discussed in mail news bug meeting. Decided to minus this bug.
Comment 37•22 years ago
|
||
move nsbeta1- to nsbeta1 . please consider this as nsbeta1+ for m1.2final
Updated•22 years ago
|
Summary: Non-ascii name of migrated address book is corrupted → Non-ascii name of migrated 4.x address book is corrupted
Comment 38•22 years ago
|
||
Gregg, can you escalate this bug to the mail team? it seems to be a nasty issue.
(this is dassi btw, not simon)
Comment 39•22 years ago
|
||
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?
Comment 40•22 years ago
|
||
i18n triage team: nsbeta1+/adt2
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.2alpha → mozilla1.4alpha
Assignee | ||
Comment 41•22 years ago
|
||
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
Assignee | ||
Updated•22 years ago
|
Attachment #117040 -
Flags: superreview?(sspitzer)
Assignee | ||
Comment 42•22 years ago
|
||
> 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.
Comment 43•22 years ago
|
||
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?
Assignee | ||
Comment 44•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Attachment #117175 -
Flags: superreview?(sspitzer)
Comment 45•22 years ago
|
||
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.
Comment 46•22 years ago
|
||
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?)
Assignee | ||
Comment 47•22 years ago
|
||
Attachment #117175 -
Attachment is obsolete: true
Comment 48•22 years ago
|
||
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 49•22 years ago
|
||
Comment on attachment 117236 [details] [diff] [review]
Fix for empty addrbook migration w/out abupgrader.
r=ccarlen
Attachment #117236 -
Flags: review?(ccarlen) → review+
Updated•22 years ago
|
Attachment #117175 -
Flags: superreview?(sspitzer)
Comment 50•22 years ago
|
||
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-
Assignee | ||
Comment 51•22 years ago
|
||
Attachment #117040 -
Attachment is obsolete: true
Comment 52•22 years ago
|
||
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+
Comment 53•22 years ago
|
||
> one fine string suggestion:
I meant, one final string suggestion.
that came out as tooting my own horn.
beep beep.
Assignee | ||
Comment 54•22 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 55•22 years ago
|
||
Comment 56•22 years ago
|
||
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+
Assignee | ||
Comment 57•22 years ago
|
||
Last fix checked in.
Reporter | ||
Comment 58•21 years ago
|
||
it is fixed, after migration of 4.x the non-ascii AB name shows correctly
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•