Closed
Bug 803835
Opened 12 years ago
Closed 12 years ago
Address Book import of CSV files messes up on quoted fields
Categories
(Thunderbird :: Address Book, defect)
Tracking
(thunderbird16 affected, thunderbird17+ fixed, thunderbird18 fixed)
RESOLVED
FIXED
Thunderbird 19.0
People
(Reporter: grawlix.computing, Assigned: mconley)
References
Details
(Keywords: regression, testcase)
Attachments
(3 files)
541 bytes,
text/plain
|
Details | |
2.38 KB,
patch
|
mconley
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
738 bytes,
patch
|
standard8
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
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•12 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•12 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.
Comment 3•12 years ago
|
||
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
Reporter | ||
Comment 4•12 years ago
|
||
The defect is not present in 15.0.1, but appears in 16.0b1.
Comment 5•12 years ago
|
||
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•12 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•12 years ago
|
||
Correction:
16.0a1, 16.0a2, 16.0aX nightlies?
Reporter | ||
Comment 8•12 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
Comment 9•12 years ago
|
||
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
Comment 10•12 years ago
|
||
This is a unit test based on the testcase attached to the bug.
Hiro, could you have a look at what has regressed here?
Updated•12 years ago
|
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → mconley
Assignee | ||
Comment 11•12 years ago
|
||
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)
Comment 12•12 years ago
|
||
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)
Assignee | ||
Comment 13•12 years ago
|
||
"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)
Updated•12 years ago
|
Attachment #674609 -
Flags: review?(mconley)
Assignee | ||
Comment 14•12 years ago
|
||
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+
Updated•12 years ago
|
Attachment #677749 -
Flags: review?(mbanner) → review+
Comment 15•12 years ago
|
||
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+
Updated•12 years ago
|
Attachment #677749 -
Flags: approval-comm-beta+
Attachment #677749 -
Flags: approval-comm-aurora+
Assignee | ||
Comment 16•12 years ago
|
||
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
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
status-thunderbird16:
--- → affected
status-thunderbird17:
--- → fixed
status-thunderbird18:
--- → fixed
OS: Windows 7 → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
You need to log in
before you can comment on or make changes to this bug.
Description
•