Address Book import of CSV files messes up on quoted fields

RESOLVED FIXED in Thunderbird 19.0

Status

Thunderbird
Address Book
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: grawlix.computing, Assigned: mconley)

Tracking

({regression, testcase})

16 Branch
Thunderbird 19.0
x86_64
All
regression, testcase

Thunderbird Tracking Flags

(thunderbird16 affected, thunderbird17+ fixed, thunderbird18 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 673563 [details]
test.csv

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010144125

Steps to reproduce:

I've attempted to import a simple CSV file into the Address Book.  The file is attached.


Actual results:

The record was imported, but the fields were not properly extracted from their quoted format in the file.  For example, instead of getting:

Acer America

...this was imported:

"Acer Americ



Expected results:

Quoted fields should not be mangled on import.
(Reporter)

Comment 1

5 years ago
This can be reliably reproduced on two systems running Thunderbird 16.0.1.  One system is running Windows 7 (64-bit), and the other, Windows XP SP3.
(Reporter)

Comment 2

5 years ago
I should also add that I believe this bug has shown up with the 16.0.1 update, as I have been routinely been making this type of import up to this point with no difficulties.
Grawlix can you try to find out when this regressed (eg using nightly builds just for that) ?
Status: UNCONFIRMED → NEW
Component: Untriaged → Address Book
Ever confirmed: true
Keywords: regression, regressionwindow-wanted, testcase
(Reporter)

Comment 4

5 years ago
The defect is not present in 15.0.1, but appears in 16.0b1.
could you try to narrow this down a bit more (see http://www.rumblingedge.com/2009/02/24/howto-find-regression-windows-through-manual-binary-search/) ?
(Reporter)

Comment 6

5 years ago
If the bug is *not* in the 15.0.1 release, but is in 16.0, I should be looking for 16a1, 16a2, 16aX nightlies?
(Reporter)

Comment 7

5 years ago
Correction: 
16.0a1, 16.0a2, 16.0aX nightlies?
(Reporter)

Comment 8

5 years ago
The bug is *not* in this daily (July 10, 2012):
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2012/07/2012-07-10-03-05-52-comm-central/thunderbird-16.0a1.en-US.win32.installer.exe

The big is in this daily (July 11, 2012):
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2012/07/2012-07-11-03-05-38-comm-central/thunderbird-16.0a1.en-US.win32.installer.exe
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2012-07-10%2004:00:00&enddate=2012-07-11%2008:00:00

This looks like a regression from bug 145293
Blocks: 145293
tracking-thunderbird17: --- → ?
Keywords: regressionwindow-wanted
Created attachment 674609 [details] [diff] [review]
Unit test from testcase

This is a unit test based on the testcase attached to the bug.

Hiro, could you have a look at what has regressed here?
tracking-thunderbird17: ? → +
Assignee: nobody → mconley
I think I'm narrowing down in on the problem.

This line here:

http://mxr.mozilla.org/comm-central/source/mailnews/import/text/src/nsTextAddress.cpp#175

Is supposed to return the number of quote characters are in a string.

Well, for this string:  

,,\"Acer America\",,,,,\"(800) 000-0000\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",,,\"Acer Americ

It's returning 38, when there are only 37 quotes in there. Not sure why though - my debugger seems to become useless when I start descending into macros.

Neil, any idea of what's going on?
Flags: needinfo?(neil)
There's a line missing from the patch in bug 145293.
>-    const char *pStart = pChar;
>-    PRInt32        fLen = 0;
>-    bool          quoted = false;
>-    if (*pChar == '"') {
>-        pStart++;
>+    PRInt32 fLen = 0;
>+    PRInt32 startPos = pos;
>+    bool    quoted = false;
>+    if (aLine[pos] == '"') {
Note that pStart++; was deleted but no startPos++; was added :-(
Flags: needinfo?(neil)
Created attachment 677749 [details] [diff] [review]
Fix

"It's a strange fate that we should suffer so much fear and doubt over something so small, such a little thing."

- Boromir, The Lord of the Rings

All import tests, including the one packaged in this bug, pass with this patch.
Attachment #677749 - Flags: review?(mbanner)
Attachment #674609 - Flags: review?(mconley)
Comment on attachment 674609 [details] [diff] [review]
Unit test from testcase

Review of attachment 674609 [details] [diff] [review]:
-----------------------------------------------------------------

Just some tailing whitespace, otherwise good.

Good stuff.

::: mailnews/import/test/unit/resources/addressbook.json
@@ +72,5 @@
> +    {
> +      "DisplayName"      : "Acer America",
> +      "Work Phone"       : "(800) 000-0000",
> +      "Organization"     : "Acer America"
> +    } 

Trailing whitespace
Attachment #674609 - Flags: review?(mconley) → review+
Attachment #677749 - Flags: review?(mbanner) → review+
Comment on attachment 674609 [details] [diff] [review]
Unit test from testcase

[Triage Comment]
We want these for 17, to fix the regression.
Attachment #674609 - Flags: approval-comm-beta+
Attachment #674609 - Flags: approval-comm-aurora+
Attachment #677749 - Flags: approval-comm-beta+
Attachment #677749 - Flags: approval-comm-aurora+
Landed on comm-central:

fix: https://hg.mozilla.org/comm-central/rev/169d801ecfb5
test: https://hg.mozilla.org/comm-central/rev/29de32c88662

comm-aurora:

fix: https://hg.mozilla.org/releases/comm-aurora/rev/b06f46e08996
test: https://hg.mozilla.org/releases/comm-aurora/rev/ca42e9f2fc61

comm-beta:

fix: https://hg.mozilla.org/releases/comm-beta/rev/bb4af80f2944
test: https://hg.mozilla.org/releases/comm-beta/rev/8b72284c58b9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-thunderbird16: --- → affected
status-thunderbird17: --- → fixed
status-thunderbird18: --- → fixed
OS: Windows 7 → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0

Updated

5 years ago
Duplicate of this bug: 812128
You need to log in before you can comment on or make changes to this bug.