483937 - Import of CSV address book silently fails

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Mitchell Mebane]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.7) Gecko/2009021910 Firefox/3.0.7 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b4pre) Gecko/20090317 Shredder/3.0b3pre

Attempting to import an address book in CSV format no longer works.  No error message is generated.

Reproducible: Always

Steps to Reproduce:
1. Open Address Book
2. Tools menu -> Import...
3. Under "type of material", select "Address Books"
4. Click Next >
5. Under "type of file", select "Text file"
6. Click Next >
7. Set the file type to CSV and select a CSV file
8. Set the fields however you like
9. Click OK
Actual Results:  
Clicking OK has no effect.  The field configuration window stays open and the address book is imported.

Expected Results:  
The field configuration window is closed and the address book is imported.

Worked in Thunderbird 3 beta 1, was broken in beta 2 and is still broken in trunk nightly as of 2009-March-17.

Nothing I change in the field configuration window seems to have any affect.

I will attach a minimal test case.

Comment 1

[Mitchell Mebane]

Attachment: Sample CSV address book

Minimal test case.  First record does NOT contain field names.  Only field is primary email.

Comment 2

[Mark Banner (:standard8)]

Confirming.

Comment 3

[Mitchell Mebane]

Hmm, small mistake in the "actual results".  That should say "address book is NOT imported".

Comment 4

[Mark Banner (:standard8)]

Attachment: Proposed fix

This should fix the problem, I've got a bit more testing to do but I think this will work.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 369286 [details] [diff] [review]
Proposed fix

I'm happy this fixes the problem.

The issue is that we added another field to the map, so when we process the field map preference where people have imported before, there's only 36 elements in the field, but there should be 37, so the UI tries to set 37 but can't do the 37th, and aborts (with the error hidden for some reason, I expect its where we are in the js stack).

This patch makes it so that we add any missing items to the fieldMap when we construct it (but by default disable them, because they are knew and we don't want to upset existing settings).

Comment 6

[[e-mail]]

Comment on attachment 369286 [details] [diff] [review]
Proposed fix

It seems to me that it would be far more straightforward to set the field map size in FieldImportOKButton().

Comment 7

[Mark Banner (:standard8)]

Attachment: Simpler Fix

Comment 8

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/4d9dc36b5b0f

Comment 9

[Mitchell Mebane]

Fixed here.  Thanks.