155491 - Address Book Export in tab/txt format, then import and it fails

Status: VERIFIED FIXED

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

Description

[Ninoschka Baca]

Branch build 2002-07-01: WinMe

Overview: Select an address book, export in txt or tab format, then import the
same file and it fails.

Steps to reproduce:
1. Select an address book which is populated with some cards
2. Select Tools|Export
3. In the Export Address Book dialog select txt (or tab) format in the pull down
menu and give the file a name (i.e. test.txt or test.tab)
4. Select Tools|Import
5. Select the Address Books radio button, select the "Text file (LDIF, tab, csv,
txt) option, Next.
6. For the Files of Type pull down select the appropriate extension so you can
see the file
7. Select the file (i.e. text.tab)

Actual Results: The Import Address Book dialog appears displaying an option to
move the fields around. The "First Name" field is present but look at the record
to the right. It displays all the data separated by a pipe sign. Select the OK
button and the name of the address book is present but it is blank, no cards
appear in the right pane.

Additional Information:
- Export ldif, import ldif = ok
- Export csv, import csv = ok

Comment 1

[Ninoschka Baca]

Marking nsbeta1 so this is considered fixed for this release.

Comment 2

[Ninoschka Baca]

Attachment: All data is separated by pipe sign next to the First Name field

Comment 3

[Ninoschka Baca]

Additional Information:
Using the same file (i.e. test.tab), I can import it into 4.x and it imports
successfully. 

Comment 4

[Ninoschka Baca]

Marking major since exporting/importing in txt or tab format is completely
broken at this point.

Comment 5

[(not reading, please use [e-mail] instead)]

I believe I caused this regression.  it has to do with a card with multiple line
in the notes field.  This regressed with my fix to get multiple line notes
fields to work with ldif.

I'm not sure why it affects txt,tab but not csv.

here's what we found:

when exporting, make sure you first card has a first name and multiple lines in
the notes field.

when you export, you'll see CRs (or LFs, not sure) in the line.

We should see how 4.x exported multiple notes lines when we exported to
txt,tab,or csv, and follow that.  (assuming it works in 4.x)

interesting note, nbaca says that when she exports from 7.0 to tab, 4.x is able
to import it.

the bug may therefore be in the *import* code, not the export.  and we need to
handle multiple line values.

my opinion is that we can ship with this, since we encourage people to use LDIF
for import and export.

Comment 6

[Cavin Song]

nbaca's exported .tab file contains 5 addresses and one of the addresses has 6 
lines of notes. The notes represented in the exported file as 5 separated lines 
with the 1st note being part of the address line itself (as the last field). So 
the file looks something like ('^' represents a tab delimiter):

John1^Smith^John1 Smith^^[e-mail] . . .  
John2^Smith^John2 Smith^^[e-mail] . . .  
John3^Smith^John3 Smith^^[e-mail] . . .  
John4^Smith^John4 Smith^^[e-mail] . . .
John5^Smith^John5 Smith^^[e-mail] . . . Note1
Note2
Ntoe3
Ntoe4
Note5
Note6

In nsTextAddress::DetermineDelim(), where we figure out what delimiter we should 
use to parse the import file, we end it up picking comma as the delimiter 
instead of tab because there are equal number of tab and comma lines in the file 
(ie, first 5 lines are tab delimited and last 5 are comma delimited). Had the 
file contained one less note then tab would have been chosen as the delimiter 
(meaning that it would have worked).

When exporting addresses with multiple lines of notes, 4.x makes all notes into 
a single note field with adjacent notes separated by two spaces. Using the above 
example, the address line for John5 Smith looks like:

 John5^Smith^John5 Smith^^[e-mail] . . . ^Note1  Notes2 ... Note6

I think we should do the same when exporting multiple notes. This will make the 
existing parsing code stays the same. All we have to do is to add code to join 
all notes together during export and to break notes apart during import.

Comment 7

[scottputterman]

Due to the fact that it only happens with tab/txt and with notes that go beyond
5 lines, I think we can live with this for Mach V rtm.  I'm not removing the
nsbeta1 since it can be used for the next release.

Comment 8

[Cavin Song]

Actually it happens when the number of address lines is less than or equal to 
the notes lines, and I think it would not happen very often.

Comment 9

[Cavin Song]

On import, 4.x does not break concatenated notes apart (ie, there will be only 
one single line for notes after import).

Comment 10

[Cavin Song]

Attachment: Proposed patch, v1

Concatenate all notes into a single line.

Comment 11

[Jean-Francois Ducarroz]

Comment on attachment 90537 [details] [diff] [review]
Proposed patch, v1

R=ducarroz

Comment 12

[(not reading, please use [e-mail] instead)]

Comment on attachment 90537 [details] [diff] [review]
Proposed patch, v1

needs work.

you are doing more work than you need to.  
In this case you are copying the value and checking IsEmpty() in all cases.
also, the column name should always be in the right case, so you don't need
to do a strcasecmp.  strcmp is enough.	also, you can use nsCRT::CR and
nsCRT::LF
which makes it more readable.

I think you want to replace

rv = importService->SystemStringFromUnicode(value.get(), valueCStr);

with:

// comment...
if (!strcmp(EXPORT_ATTRIBUTES_TABLE[i].abColName, kNotesColumn)) {
  nsAutoString tempStr(value.get());
  if (!tempStr.IsEmpty()) {
    tempStr.ReplaceChar(nsCRT::CR, ' ');
    tempStr.ReplaceChar(nsCRT::LF, ' ');
  }
  rv = importService->SystemStringFromUnicode(tempStr.get(), valueCStr);
}
else {
  rv = importService->SystemStringFromUnicode(value.get(), valueCStr);
}

Comment 13

[Cavin Song]

Attachment: Proposed patch, v2

Incorporated Seth's comments.

Comment 14

[(not reading, please use [e-mail] instead)]

Comment on attachment 90567 [details] [diff] [review]
Proposed patch, v2

sr=sspitzer

Comment 15

[Jean-Francois Ducarroz]

Comment on attachment 90567 [details] [diff] [review]
Proposed patch, v2

R=ducarroz

Comment 16

[Jean-Francois Ducarroz]

Comment on attachment 90567 [details] [diff] [review]
Proposed patch, v2

R=ducarroz

Comment 17

[Cavin Song]

Fix checked in to the trunk.

Comment 18

[Ninoschka Baca]

Reopening because I found an additional problem on Mac 9.1 related to notes.

Branch build 2002-07-12: Mac 9.1
Problem: On the mac the original cards are present but extra cards are created.
Steps to reproduce:
1. Create an address book with three cards 
2. In one of the cards includes notes (3 lines of text) such as:
"one
 two
 three" 
3. Export in text format (txt, tab or csv)
4. Import

Actual Results: The original cards are present but 2 more cards are present. The
first extra card has "two" for the First Name and the other extra card has
"three" for the First Name. Interesting that the original card w/notes no longer
has notes.

Comment 19

[Cavin Song]

This was fixed only on the trunk but not the branch.

But there's another problem with the latest trunk build. The problem is that we 
export fields that are not supposed to be exported. For example, fields like 
work phone type, default email column, etc still show up in the exported .tab 
and/or .csv files (as fields with no value). The fix is to not write out the 
delimiter associated with the fields that are not supposed to be exported. A 
patch is coming.

Comment 20

[Cavin Song]

Attachment: Proposed patch, v1

Don't write out the delimiter if the field is to be ignored (for export). Also,
cellular number type should not be exported.

Comment 21

[Jean-Francois Ducarroz]

Comment on attachment 100779 [details] [diff] [review]
Proposed patch, v1

R=ducarroz

Comment 22

[(not reading, please use [e-mail] instead)]

Comment on attachment 100779 [details] [diff] [review]
Proposed patch, v1

sr=sspitzer

thanks for cleaning up my bad code.

Comment 23

[Cavin Song]

Fix checked in to the trunk.

Comment 24

[Cavin Song]

Marking fixed. Please reopen it if this needs to be on the branch.

Comment 25

[Ninoschka Baca]

Trunk build 2002-11-08-13: WinMe
Trunk build 2002-11-21: Linux 8, Mac 10.1.3
Verified Fixed.