953135 - convertTo("vcard") method of nsIAbCard does not export e-mail address

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Lukasz Stelmach]

Steps to reproduce:

1. Find a contact in an address book with a WorkCountry unset.
2. Access the contact as an instance of nsIAbCard from JS code.
3. Call card.convertTo("vcard").

Actual results:

Primary e-mail address is missing from the vCard.

Expected results:

Primary e-mail is exported to vCard.

Reason:

Invalid check for rv after calling

(void)GetPrimaryEmail(str);

http://mxr.mozilla.org/comm-central/source/mailnews/addrbook/src/nsAbCardProperty.cpp#631

Comment 1

[Lukasz Stelmach]

Attachment: What seems to be an obvious fix (option #1)

I haven't tested the patch.

Comment 2

[Lukasz Stelmach]

Attachment: What seems to be an obvious fix (option #2)

I haven't tested the patch.

Comment 3

[[e-mail]]

Comment on attachment 8351486 [details] [diff] [review]
What seems to be an obvious fix (option #1)

So, it does work if your card has a work country ;-)

I prefer this patch because it's in the style of the display name, last name and first name.

Comment 4

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v1

Since the original patch author doesn't seem to be around, I am uploading the fix so that it gets in.

Thanks.

Comment 5

[[e-mail]]

(In reply to comment #3)
> So, it does work if your card has a work country ;-)

(In reply to Lukasz Stelmach)
> 1. Find a contact in an address book with a WorkCountry unset.

Suyash pointed out that you already said that. Sorry about that.

Comment 6

[[e-mail]]

Comment on attachment 8573325 [details] [diff] [review]
Patch v1

># User Suyash Agarwal <[e-mail]>
Shouldn't this credit the original author? ;-)

Comment 7

[Suyash Agarwal (:sshagarwal)]

(In reply to [e-mail] from comment #6)
> Comment on attachment 8573325 [details] [diff] [review]
> Patch v1
> 
> ># User Suyash Agarwal <[e-mail]>
> Shouldn't this credit the original author? ;-)

I wrote that in the commit message. Didn't know we can do it here as well.
Will just make the change. sorry

Comment 8

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch for check-in

Comment 9

[Lukasz Stelmach]

I am always around, somewhere. I just can't be around everywhere (-;

BTW. Does being the assignee mean I need to take some actions now?

Comment 10

[Suyash Agarwal (:sshagarwal)]

(In reply to Lukasz Stelmach from comment #9)
> I am always around, somewhere. I just can't be around everywhere (-;
> 
> BTW. Does being the assignee mean I need to take some actions now?

No, nothing needs to be done :)

Comment 11

[Kent James (:rkent)]

Comment on attachment 8573334 [details] [diff] [review]
Patch for check-in

Checked in https://hg.mozilla.org/comm-central/rev/2bc6eb521e4b

We'll consider for comm-aurora after nightly bake.

Comment 12

[Kent James (:rkent)]

Comment on attachment 8573334 [details] [diff] [review]
Patch for check-in

https://hg.mozilla.org/releases/comm-aurora/rev/cd8b6a293cab