Closed
Bug 261882
Opened 20 years ago
Closed 15 years ago
Import & Export to/from text files (CSV,TAB) fails to include "Screenname" field
Categories
(MailNews Core :: Address Book, defect)
MailNews Core
Address Book
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b2
People
(Reporter: tommy.joe, Assigned: bisi)
References
Details
(Keywords: dataloss, helpwanted)
Attachments
(1 file, 5 obsolete files)
9.06 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10 When importing an address book from a text based file, there is no option to match the screenname field to any of the fields present in the data file. Also, when an address book containing data entries in the screen name field, they are not included in the exported text file. Reproducible: Always Steps to Reproduce: 1. Choose Import from the Address Book Tools menu 2. Select "Address Books" 3. Select "Text File" 4. Choose a path to an appropriate file 5. The dialog requesting the user to match the field names with the individual data items does not have a field for "Screenname" Also 1. Select an address book containing data entries in the "Screenname" field 2. Choose "Export" from the "Tools" menu 3. Export to a CSV or TAB file 4. Open the file in a text editor, and the screenname data is gone
Updated•19 years ago
|
Summary: Import & Export to/from text files (LDIF,CSV,TAB) fails to include "Screenname" field → Import & Export to/from text files (LDIF,CSV,TAB) fails to include "Screenname" field
Comment 1•19 years ago
|
||
Confirming bug on tb trunk version 1.0+ (20050704). Note that LDIF import is being fixed in bug 177968 - export was already working. Hence I've removed ldif from the summary.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Import & Export to/from text files (LDIF,CSV,TAB) fails to include "Screenname" field → Import & Export to/from text files (CSV,TAB) fails to include "Screenname" field
Updated•19 years ago
|
Assignee: mscott → nobody
Component: Address Book → MailNews: Address Book
OS: Windows 2000 → All
Product: Thunderbird → Core
QA Contact: addressbook
Hardware: PC → All
Version: unspecified → Trunk
Updated•19 years ago
|
Keywords: helpwanted
Comment 2•18 years ago
|
||
*** Bug 353331 has been marked as a duplicate of this bug. ***
Comment 3•16 years ago
|
||
Marking as blocking-thunderbird 3, since it's a dataloss bug.
Flags: blocking-thunderbird3+
Keywords: dataloss
Updated•16 years ago
|
Product: Core → MailNews Core
Assignee | ||
Comment 4•16 years ago
|
||
Taking. I have a patch in my tree, that works with trunk builds. However, the real problem here is backwards compatibility. I'll look around, If I can find a reasonable solution.
Assignee: nobody → bisi
Comment 5•15 years ago
|
||
Žiga, are you still willing to do the patch for this bug? I'm not sure backwards compatibility would be too much of an issue as users can just choose not to import certain fields.
Assignee | ||
Comment 6•15 years ago
|
||
Hey Mark, thanks for stopping by. :) Actually, my development laptop died a couple of months ago and I didn't get to buy a new one. But I'm also a bad bug owner. ;) Anyway, I didn't lose my data, so I'm attaching a wip patch (there's a small merge conflict in import_helper.js and it hasn't been updated for at least two months). Well, I'm not sure if that's the right approach - should a "new field" be placed where it actually belongs or at the end of the field map?
Comment 7•15 years ago
|
||
Žiga, sorry for not getting back to you earlier. - {kScreenNameProperty}, + {kScreenNameProperty, 2106}, {kPreferMailFormatProperty}, {kLastModifiedDateProperty}, - {kWorkPhoneProperty, 2106}, + {kWorkPhoneProperty, 2107}, {kWorkPhoneTypeProperty}, - {kHomePhoneProperty, 2107}, + {kHomePhoneProperty, 2108}, {kHomePhoneTypeProperty}, - {kFaxProperty, 2108}, + {kFaxProperty, 2109}, I'd definitely like to avoid renumbering the strings - that will put extra work on the localisers, you can just make the screen name string to be 2136 and then use the map to map kScreenNameProperty to 2136. case 6: + rv = card->GetPropertyAsAString(kScreenNameProperty, value); + break; + case 7: rv = card->GetPropertyAsAString(kWorkPhoneProperty, value); break; - case 7: + case 8: rv = card->GetPropertyAsAString(kHomePhoneProperty, value); If this is doing what I think its doing (inserting the screen name property in position '6'), then I don't think we want to quite do it that way. There is a mailnews.import.text.fieldmap that isn't defined by default, but you could use that to put the screen name in the right place for new users. For existing users it would leave the existing order unaffected so if they are already set up in a particular manner then they won't have all their fields shifted. To do this, insert mailnews.import.text.fieldmap into mailnews/mailnews.js and set it to something like: +0,+1,+2,+3,+36,+4, etc. I think that would be a better solution and cut down the size of the patch. Note: I'm taking this off the blocking TB 3 list as I doubt we'd hold the release for it because we've shipped like this before. However, I'd very much like to get the fix in - if you can't get it in soon, let me know and I'll see if I can find someone to pick it up.
Flags: wanted-thunderbird3+
Flags: blocking-thunderbird3-
Flags: blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b2
Assignee | ||
Comment 8•15 years ago
|
||
Wow, didn't know about the mailnews.import.text.fieldmap pref. This makes things much easier. I'll update the patch soon and request a review. Thanks Mark! :)
Assignee | ||
Comment 9•15 years ago
|
||
Attachment #357247 -
Attachment is obsolete: true
Attachment #358636 -
Flags: review?(bugzilla)
Assignee | ||
Comment 10•15 years ago
|
||
Argh, this is the right patch.
Attachment #358636 -
Attachment is obsolete: true
Attachment #358640 -
Flags: review?(bugzilla)
Attachment #358636 -
Flags: review?(bugzilla)
Assignee | ||
Updated•15 years ago
|
Attachment #358640 -
Attachment is patch: true
Attachment #358640 -
Attachment mime type: application/octet-stream → text/plain
Comment 11•15 years ago
|
||
Comment on attachment 358640 [details] [diff] [review] updated patch Thanks for updating the patch, from a quick glance it looks good, but one thing I did notice: +pref("mailnews.import.text.fieldmap", "+0+1+2+3+36+4+5+6+7+8+9+10+11+12+13+14+15+16+17+18+19+20+21+22+23+24+25+26+27+28+29+30+31+32+33+34+35"); Shouldn't they have commas in between them? My pref currently shows: +0,+1,+2,+3,-4,-5,-6,-7,-8,-9,-10,-11,-12,-13,-14,-15,-16,-17,-18,-19,-20,-21,-22,-23,-24,-25,-26,-27,-28,-29,-30,-31,-32,-33,-34,-35, I'll do the full review on Monday.
Assignee | ||
Comment 12•15 years ago
|
||
Oh yes, and the position of "36" was also wrong in the previous patch. Here's the corrected patch.
Attachment #358640 -
Attachment is obsolete: true
Attachment #358650 -
Flags: review?(bugzilla)
Attachment #358640 -
Flags: review?(bugzilla)
Comment 13•15 years ago
|
||
Comment on attachment 358650 [details] [diff] [review] updated patch + case 36: + rv = database->addAimScreenName(row, pVal); + break; It should be AddAimScreenName (i.e. captial A). Otherwise this works nicely. r=me (you'll need an sr as well).
Attachment #358650 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 14•15 years ago
|
||
Thanks Mark. Asking Dan for a sr.
Attachment #358650 -
Attachment is obsolete: true
Attachment #358839 -
Flags: superreview?(dmose)
Comment 15•15 years ago
|
||
Comment on attachment 358839 [details] [diff] [review] A updated patch >diff --git a/mail/locales/en-US/chrome/messenger/importMsgs.properties b/mail/locales/en-US/chrome/messenger/importMsgs.properties >--- a/mail/locales/en-US/chrome/messenger/importMsgs.properties >+++ b/mail/locales/en-US/chrome/messenger/importMsgs.properties >@@ -273,16 +273,21 @@ 2133=Custom 3 > ## @loc None > 2134=Custom 4 > > # Description: Address book field name > ## @name IMPORT_FIELD_DESC_END > ## @loc None > 2135=Notes > >+# Description: Address book field name >+## @name IMPORT_FIELD_DESC_END >+## @loc None >+2136=Screen Name The @name comment above 2135 should change to IMPORT_FIELD_DESC, since it's no longer the end. sr=dmose with this fixed in both versions of importMsgs.properties. Thanks for the patch!
Attachment #358839 -
Flags: superreview?(dmose) → superreview+
Assignee | ||
Comment 16•15 years ago
|
||
Nits fixed. Adding the checkin-needed keyword.
Attachment #358839 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Comment 17•15 years ago
|
||
Patch checked in: http://hg.mozilla.org/comm-central/rev/c01858f3a47b Many thanks for fixing this Žiga.
You need to log in
before you can comment on or make changes to this bug.
Description
•