Thunderbird 68 cannot import CSV address book on first attempt
Categories
(Thunderbird :: Address Book, defect)
Tracking
(thunderbird_esr6870+ fixed, thunderbird70 fixed, thunderbird71 fixed)
People
(Reporter: earlgreypicard, Assigned: darktrojan)
References
(Regression)
Details
(Keywords: regression)
Attachments
(5 files)
214 bytes,
text/plain
|
Details | |
51.38 KB,
image/jpeg
|
Details | |
67.52 KB,
image/jpeg
|
Details | |
1.26 KB,
patch
|
darktrojan
:
review-
|
Details | Diff | Splinter Review |
2.61 KB,
patch
|
mkmelin
:
review+
jorgk-bmo
:
feedback+
jorgk-bmo
:
approval-comm-beta+
jorgk-bmo
:
approval-comm-esr68+
|
Details | Diff | Splinter Review |
A report was posted on the MozillaZine.jp forum that CSV exported from Windows Live Mail contacts cannot be imported into the Thunderbird's Addressbook.
I noticed that one of the CSV field names exported from Windows Live Mail is "ι»εγ‘γΌγ« γ’γγ¬γΉ". So I tried importing a CSV with field spaces removed into Thunderbird and it was successful.
Steps to reproduce:
- Download Sample Contacts.csv.
- Open Thunderbird's Address Book.
- Select Tools > Import... > Addrerss Books > Text file (LDIF, .tab, .csv, .txt)
- Open Sample Contacts.csv to import it.
Actual results:
No contacts are imported(See Addressbook(TB68).jpg).
Expected results:
Like Thunderbird 52 (See Addressbook(TB52).jpg), 5 contacts are imported.
Regression Infomation:
The result of executing mozregression-gui is as follows
-
last good
build_url: https://archive.mozilla.org/pub/thunderbird/nightly/2018/07/2018-07-18-10-53-45-comm-central/thunderbird-63.0a1.en-US.win64.zip
changeset: 8dab948a10f073a46f13f55f94d1f6514c7360ac
pushlog_url: https://hg.mozilla.org/comm-central/pushloghtml?fromchange=8dab948a10f073a46f13f55f94d1f6514c7360ac&tochange=4f12d77b4f9b6adaf06615c1c8cdc14de836dc1a -
first bad
build_url: https://archive.mozilla.org/pub/thunderbird/nightly/2018/07/2018-07-20-10-01-33-comm-central/thunderbird-63.0a1.en-US.win64.zip
changeset: 4f12d77b4f9b6adaf06615c1c8cdc14de836dc1a
pushlog_url: https://hg.mozilla.org/comm-central/pushloghtml?fromchange=547144f5596c1a146b208d68d93950a6313080ca&tochange=4f12d77b4f9b6adaf06615c1c8cdc14de836dc1a
Reporter | ||
Comment 1•5 years ago
|
||
Reporter | ||
Comment 2•5 years ago
|
||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Right, I tried
First Name,Last Name,E-mail Address
Anita,Jorgensen,Anita@contoso.com
Anne-Mette,Olesen,Anne-Mette@contoso.com
Dorena,Paschke,DorenaP@contoso.com
Kemal,Celik,Kemal@contoso.com
Shiori,Inoue,Shiori@contoso.com
and nothing gets imported.
But that input data is wrong. The "E-mail Address" contains a space, as the bug title says. It works OK with First Name,Last Name,E-mailAddress.
I don't think we want to invest too much work here. The links in comment #0 don't work.
Alice, can you please give us the correct range and then we'll take a look.
Updated•5 years ago
|
![]() |
||
Comment 4•5 years ago
|
||
What is problem?
I can import as expected using attached Sample Contacts.csv on Tb71.0a1(20190923003205) Windows10.
Comment 5•5 years ago
|
||
Interesting. I can't, it doesn't work at all, nothing is imported. I was trying an en-US build of TB 68.1.1.
![]() |
||
Comment 6•5 years ago
•
|
||
(In reply to Alice0775 White from comment #4)
What is problem?
I can import as expected using attached Sample Contacts.csv on Tb71.0a1(20190923003205) Windows10.
Wait.
Aha, I can reproduce with new profile.
However, After delete the failed Address Book("Sample Contacts"). And try again, it works as expected. So It seems to be a bug of some initialization function(i.e, the initial state of checkbox in the dialog isn't being set properly).
Anyway, I will continue to investigate regression range.
![]() |
||
Comment 7•5 years ago
|
||
Regression window(from local cached mozregression):
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=48373a3601ff8eab496b2a744af7aaa1ee0cd9dc&tochange=a7d0257d9621817b6e6fc3094ebeec93097bc44d
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1174739835690f7f4024a913794e5cf35d1524a2&tochange=5a8107262015714d2907a85abc24c847ad9b32d2
Comment 8•5 years ago
|
||
Thanks, Alice. I don't see anything specific in that range. Looks like we have to debug it. But since it works without the space in the field name, there's not a lot of pressure, I assume.
![]() |
||
Comment 9•5 years ago
|
||
Interesting,
Import AB first time, Initial state of checkboxes are all on. But if turning off all checkboxes, the import AB succeeds. --- so, it seems to indicate reverse state of the checkbox.
(In reply to Jorg K (GMT+2) from comment #8)
Thanks, Alice. I don't see anything specific in that range. Looks like we have to debug it. But since it works without the space in the field name, there's not a lot of pressure, I assume.
I think the space is not related to this. I tried dale the space in CSV. But same thing happens.
Comment 10•5 years ago
|
||
Import AB first time, Initial state of checkboxes are all on. But if turning off all checkboxes, the import AB succeeds.
OK, this makes more sense. I also saw strange checkbox behaviour on the first import. So this is related to the listbox/richlistbox changes in bug 1475817. Oh joy, more de-XBL or de-XUL bustage:
a7d0257d9621817b6e6fc3094ebeec93097bc44d Geoff Lankow β Bug 1475817 - Part 11: Convert simple <listbox> to <richlistbox> in mailnews/import. r=Paenglab,jorgk
Magnus, who would be a good resource for this?
![]() |
||
Comment 11•5 years ago
|
||
FYI, it works as expected if add a line as follows;
--- a/mailnews/import/content/fieldMapImport.js 2010-01-01 00:00:00.000000000 +0900
+++ b/mailnews/import/content/fieldMapImport.js 2019-09-26 22:37:54.353751400 +0900
@@ -93,16 +93,17 @@
var checkboxCell = document.createXULElement("hbox");
checkboxCell.setAttribute("style", "width: var(--column1width)");
let checkbox = document.createXULElement("checkbox");
checkbox.addEventListener("click", cellClicked);
if (on) {
checkbox.setAttribute("checked", "true");
}
checkboxCell.appendChild(checkbox);
+ checkboxCell.setAttribute("checked", on);
var firstCell = document.createXULElement("label");
firstCell.setAttribute("style", "width: var(--column2width)");
firstCell.setAttribute("value", name);
var secondCell = document.createXULElement("label");
secondCell.setAttribute("class", "importsampledata");
secondCell.setAttribute("flex", "1");
Comment 12•5 years ago
|
||
Thanks, Alice. I'll take a look later. I don't think that setting "checked" on the checkboxCell, which is a hbox, is correct. But the solution will be close to the code you pointed out.
Comment 13•5 years ago
|
||
I'll let you review this. Surely seeing a listbox will make you all nostalgic ;-)
https://hg.mozilla.org/comm-central/rev/a7d0257d9621817b6e6fc3094ebeec93097bc44d#l2.21
When you do the import, the windows isn't refreshed with the imported content, you need to select another AB and then the imported one. That's for another bug.
You can land the patch if you want (assuming r+).
![]() |
||
Comment 14•5 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #12)
Thanks, Alice. I'll take a look later. I don't think that setting "checked" on the checkboxCell, which is a hbox, is correct. But the solution will be close to the code you pointed out.
umm, something tricky,
gListbox.selectedItem.firstChild / gListbox.getItemAtIndex(i).firstChild is pointing at <hbox style="width: var(--column1width)">
and finally the check status is gListbox.getItemAtIndex(i).firstChild.getAttribute("checked") in FieldImportOKButton method.
Comment 15•5 years ago
|
||
You're saying that let on = gListbox.getItemAtIndex(i).firstChild.getAttribute("checked");
isn't right since is a hbox and not the checkbox?
I'm really not a front-end expert. I'll let Geoff deal with it. Does my patch not work for you?
Geoff, feel free to r- and come up with a better patch. I'm just looking at regressions and trying to get them fixes as well as possible.
Assignee | ||
Comment 16•5 years ago
|
||
Comment on attachment 9096187 [details] [diff] [review] 1584026-fix-AB-import.patch This wouldn't achieve anything. :-) `checkbox` is already unchecked on account of being freshly created.
Assignee | ||
Comment 17•5 years ago
|
||
This uses the result of the actual checkbox, not the cell containing it. I've also killed off the listener for toggling the checkboxes, since they are capable of doing that themselves.
Comment 18•5 years ago
|
||
This wouldn't achieve anything. :-)
checkbox
is already unchecked on account of being freshly created.
Hmm, well, the observed behaviour was that on first run all boxes were checked and the patch fixed that. But that may be coincidental ;-)
Comment 19•5 years ago
|
||
Comment on attachment 9096798 [details] [diff] [review] 1584026-field-map-checkboxes-1.diff Review of attachment 9096798 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, r=mkmelin
Comment 20•5 years ago
|
||
Comment on attachment 9096798 [details] [diff] [review] 1584026-field-map-checkboxes-1.diff Sorry, but this doesn't work (unlike my patch). On first input with the sample AB attached here, you now have the first three rows checked instead of all rows without the patch. Nothing is imported. Works OK the second time.
Comment 23•5 years ago
|
||
Comment on attachment 9096798 [details] [diff] [review] 1584026-field-map-checkboxes-1.diff Building with the patch helped ;-) - Sorry about the noise.
Comment 24•5 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/ba107919a387
Fix broken field map dialog checkboxes for address book import. r=mkmelin DONTBUILD
Updated•5 years ago
|
Comment 25•5 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/5dea3f78c652 Follow-up: Fix linting. rs=white-space-only DONTBUILD
Comment 26•5 years ago
|
||
TB 70 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/21607b2ee300199a8d2474f2f507b2b19908b875 (merged patch)
Comment 27•5 years ago
|
||
TB 68.1.2 or TB 68.2:
https://hg.mozilla.org/releases/comm-esr68/rev/42ed08c599dfb41a076d21e5d2cd37023b9e99cd
Description
•