Closed Bug 483937 Opened 15 years ago Closed 15 years ago

Import of CSV address book silently fails

Categories

(Thunderbird :: Address Book, defect)

x86
Windows Vista
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: mitchell.mebane, Assigned: standard8)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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.
Minimal test case.  First record does NOT contain field names.  Only field is primary email.
Confirming.
Assignee: nobody → bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3+
Keywords: regression
Target Milestone: --- → Thunderbird 3.0b3
Hmm, small mistake in the "actual results".  That should say "address book is NOT imported".
Attached patch Proposed fix (obsolete) — Splinter Review
This should fix the problem, I've got a bit more testing to do but I think this will work.
Status: NEW → ASSIGNED
Whiteboard: [has draft patch]
Whiteboard: [has draft patch] → [has patch][needs review neil]
Attachment #369286 - Flags: superreview?(neil)
Attachment #369286 - Flags: review?(neil)
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 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().
Attachment #369286 - Flags: superreview?(neil)
Attachment #369286 - Flags: superreview-
Attachment #369286 - Flags: review?(neil)
Attached patch Simpler FixSplinter Review
Attachment #369286 - Attachment is obsolete: true
Attachment #369485 - Flags: superreview?(neil)
Attachment #369485 - Flags: review?(neil)
Attachment #369485 - Flags: superreview?(neil)
Attachment #369485 - Flags: superreview+
Attachment #369485 - Flags: review?(neil)
Attachment #369485 - Flags: review+
Checked in: http://hg.mozilla.org/comm-central/rev/4d9dc36b5b0f
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][needs review neil]
Fixed here.  Thanks.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: