368630 - Importer (Win Eudora, Address Book). Variouos fields not mapped to Penelope Address Book fields

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Matt Dudziak]

The problem is that various Eudora Address Book fields are not mapped to Penelope Address Book fields, even when an appropriate field is available in the Penelope Address Book.

To reproduce:

- In Eudora, create an Address Book entry and enter values for all available fields (or at least for Work Fax, work Mobile, "Other" email addresses, "Other" phone number, "Other" web page)
- Import Address Book into Penelope 
- View card in Penelope 
- Note that the 'Other" email address was not mapped to Penelope's "Additional email" field.
- Note there is no corresponding field in Penelope for the rest of the fields listed above. This data is imported to the "notes" in Penelope...

Comment 1

[Geoffrey C. Wenger]

Attachment: Patch that fixes bug. Note patch was not created with CVS.

* First email address from Eudora's "Other Email" is now mapped to TB's "Additional Email" field. Subsequent email addresses from Eudora's "Other Email" are now formatted nicely in the Notes field with the label "Other Email:".
* Eudora's "Other Phone" and "Other Web" are now formatted nicely in the notes field, labelled appropriately.
* Importer now recognizes which location for a contact (home or work) is marked primary and imports primary mobile and fax numbers into TB's "Mobile" and "Fax" fields (previously home fax and mobile numbers were imported; work fax and mobile numbers were left as raw data in the notes field). The secondary (non-primary) mobile and fax numbers are now imported into the "Custom 1" and "Custom 2" fields respectively. Numbers are labelled appropriately with the following labels: "Personal Mobile", "Work Mobile", "Home Fax", "Work Fax".

Comment 2

[Scott MacGregor]

Comment on attachment 262175 [details] [diff] [review]
Patch that fixes bug. Note patch was not created with CVS.

Can this just be a \n instead of \x0D\x0A?

+        additionalEmail.ReplaceSubstring( "\x03", "\x0D\x0A");

ditto for the other occurrences. 

I didn't know about .Compare:

primaryLocation.Compare("home")

I've always used primaryLocation.EqualsLiteral("home")

I know this importer code was originally written very poorly so it has lots of bad examples in it...I think you can do this to optimize string buffer useage even though the original importer code doesn't use this pattern:

make both of these nsString objects
+    PRUnichar       *pFormat;
+    PRUnichar       *pCustomData;

then do:

pFormat.Adopt(nsEudoraStringBundle::GetStringByID(stringID, bundle));
pCustomData.Adopt(nsTextFormatter::smprintf( pFormat.get(), NS_ConvertASCIItoUTF16(secondaryMobile).get() )

you can then remove these two lines:

+      nsTextFormatter::smprintf_free(pCustomData);
+      nsEudoraStringBundle::FreeString(pFormat);


I like the patch! sr=mscott if you wouldn't mind making the suggestions I listed above.

Comment 3

[Geoffrey C. Wenger]

Attachment: Patch that improves on previous patch that fixed the bug (i.e. must apply original patch first). Note patch was not created with CVS.

* Builds on patch "bug-368630.patch". Contains improvements Scott suggested in response to the original patch. This patch does NOT stand on its own. It must be applied after "bug-368630.patch".
* Changed instances of \x0D\x0A to \n
* Changed calls to nsCString::Compare (which is deprecated) to instead call LowerCaseEqualsASCII
* Switched to use of nsString and Adopt (and automatic freeing) rather than naked pointers and calls to free the pointers

Comment 4

[Geoffrey C. Wenger]

(In reply to comment #2)
> (From update of attachment 262175 [details] [diff] [review])
> Can this just be a \n instead of \x0D\x0A?
> 
> +        additionalEmail.ReplaceSubstring( "\x03", "\x0D\x0A");
> 
> ditto for the other occurrences.

\n seems to work fine, so I made that change. Please note though that most of the use of \x0D\x0A precedes my original patch.



> I didn't know about .Compare:
> 
> primaryLocation.Compare("home")
> 
> I've always used primaryLocation.EqualsLiteral("home")

EqualsLiteral is available with nsAutoString, but I'm using nsCString. In checking it out Compare is deprecated, so I switched the code to use LowerCaseEqualsASCII instead.


> I know this importer code was originally written very poorly so it has lots of
> bad examples in it...I think you can do this to optimize string buffer useage
> even though the original importer code doesn't use this pattern:
> 
> make both of these nsString objects
> +    PRUnichar       *pFormat;
> +    PRUnichar       *pCustomData;
> 
> then do:
> 
> pFormat.Adopt(nsEudoraStringBundle::GetStringByID(stringID, bundle));
> pCustomData.Adopt(nsTextFormatter::smprintf( pFormat.get(),
> NS_ConvertASCIItoUTF16(secondaryMobile).get() )
> 
> you can then remove these two lines:
> 
> +      nsTextFormatter::smprintf_free(pCustomData);
> +      nsEudoraStringBundle::FreeString(pFormat);

Done. Thanks much for this and all above suggestions.

Comment 5

[David :Bienvenu]

Attachment: cumulative patch

this is what I'm going to land, plus the properties changes, to both mail and suite dirs...

Comment 6

[Scott MacGregor]

Comment on attachment 263762 [details] [diff] [review]
Patch that improves on previous patch that fixed the bug (i.e. must apply original patch first). Note patch was not created with CVS.

belated flag cleanup, this patch has already landed on the trunk.