Last Comment Bug 879435 - my vCard forgets values (preferred email format and jabber id)
: my vCard forgets values (preferred email format and jabber id)
Status: VERIFIED FIXED
:
Product: MailNews Core
Classification: Components
Component: Address Book (show other bugs)
: 17
: x86 Linux
: -- minor (vote)
: Thunderbird 24.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-04 12:51 PDT by Justus Seifert
Modified: 2016-07-12 07:33 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot_bug_vcard_thunderbird.png (17.25 KB, image/png)
2013-06-04 12:51 PDT, Justus Seifert
no flags Details
patch (5.32 KB, patch)
2013-06-05 13:41 PDT, :aceman
standard8: review-
Details | Diff | Splinter Review
patch v2 (3.98 KB, patch)
2013-06-10 14:22 PDT, :aceman
standard8: review+
Details | Diff | Splinter Review

Description Justus Seifert 2013-06-04 12:51:17 PDT
Created attachment 758121 [details]
screenshot_bug_vcard_thunderbird.png

User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130514164240

Steps to reproduce:

I opened my vCard in the Accounts settings and changed the value of "Prefers to receive messages fomated as:" and "Jabber ID:". I clicked ok and after that opened the vcard editor again. 


Actual results:

My settings were not present. In fact the drop down for the format did not show any viable option but instead was resized to a very flat slab without text.  the Jabber ID


Expected results:

The drop down menu should have showed text with my selection from earlier and the field for the jabber id should have contained the jabber id I entered.
Comment 1 :aceman 2013-06-05 01:38:33 PDT
Confirming and taking.
Comment 2 Mark Banner (:standard8) 2013-06-05 01:44:51 PDT
"Prefers to receive messages fomated as:" shouldn't actually be displayed on the Vcard - that's a Thunderbird address book specific thing and there's nothing in vcard that would let that be recorded (or even interpreted at the other end).

Jabber id & other instant messaging ids there might be, but we'd need to check the vcard 2.1 spec (which we vaguely implement), and then see if we can implement support for those fields.
Comment 3 :aceman 2013-06-05 02:59:05 PDT
Thanks for the hints.
I have not found any mention of IM/chat nicks in the spec at http://www.imc.org/pdi/vcard-21.txt .

So I try to hide the mentioned unsupported fields.
Comment 4 Justus Seifert 2013-06-05 05:51:50 PDT
@aceman: thank you.  one thing to note: thunderbird supports the preferred mail format in .vcf files through an extension.  i dont know if this is legit in vcf but it might work.  the string in the file looks like this: 
x-mozilla-html:FALSE
Comment 5 :aceman 2013-06-05 06:00:10 PDT
Yes, that is true. And the bug is that we need to convert from FALSE/TRUE to values of 1 or 2 for the dropdown.

Justus, have you tried that other Thunderbird can receive this setting from a sent vCard and use it properly? If yes, we could keep it in.

Standard8, could we keep the field in as it is clearly marked as an extension? I have tested that the field (x-mozilla-html) is stored in the vcard in prefs.js. we just fail to show it in the dialog at later read attempt.
Comment 6 rsx11m 2013-06-05 06:05:20 PDT
> Product/Component: MailNews Core :: Address Book

Hmm, I thought that vCard is kept in nsIMsgIdentity, hence Account Management?
Comment 7 Justus Seifert 2013-06-05 06:11:47 PDT
before i test weather thunderbird can receive proper data from this extension in vcards i would like to request that a similar extension for xmpp is added.  something like this:
x-mozilla-xmppid:example@example.com
Comment 8 Justus Seifert 2013-06-05 06:16:06 PDT
Ok thunderbird seems to understand its own vcf extension:  when i opened the vcard i was shown the same messed up drop down menu.
it appears all that is missing is indeed the conversion to the correct values for display.
Comment 9 :aceman 2013-06-05 13:21:33 PDT
(In reply to Justus Seifert from comment #7)
> before i test weather thunderbird can receive proper data from this
> extension in vcards i would like to request that a similar extension for
> xmpp is added.  something like this:
> x-mozilla-xmppid:example@example.com

Please file this as a new enhancement request bug.

In this bug I hide the chat fields so that it does not look like we support them already.
Comment 10 :aceman 2013-06-05 13:41:09 PDT
Created attachment 758778 [details] [diff] [review]
patch

So I hide the Chat fields but preserve the "prefers HTML" dropdown, just decode it properly. I also added Seamonkey version but as I could not test it I don't know whether it is needed.
Comment 11 Mark Banner (:standard8) 2013-06-10 01:52:14 PDT
Comment on attachment 758778 [details] [diff] [review]
patch

Review of attachment 758778 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/addrbook/content/abCardOverlay.js
@@ +499,5 @@
>    var popup = document.getElementById("PreferMailFormatPopup");
> +  if (popup) {
> +    let format = cardproperty.getProperty("PreferMailFormat", "");
> +    popup.value =
> +      (format == "TRUE") ? Components.interfaces.nsIAbPreferMailFormat.html : (

This is the wrong place to do this conversion, as it means that we'll potentially have nsAbCardProperty objects in the system with two types of value for the nsIAbPreferMailFormat.

I think this conversion should be done somewhere around http://hg.mozilla.org/comm-central/annotate/0c54a5703d29/mailnews/addrbook/src/nsAbManager.cpp#l1120 in the AB itself.

@@ +537,5 @@
>  // by vCard so the user does not try to edit them.
>  function HideNonVcardFields()
>  {
>    document.getElementById("homeTabButton").hidden = true;
> +  document.getElementById("chatTabButton").hidden = true;

Just to note, this is fine. As you mentioned, handling the other extensions for the ids should be done in a different bug.
Comment 12 :aceman 2013-06-10 02:12:45 PDT
Thanks. So invert the logic from http://hg.mozilla.org/comm-central/file/3c7cb0989fef/mailnews/addrbook/src/nsAbCardProperty.cpp#l512, where only kPreferMailFormatProperty is handled as Uint32, others are String.
convertNameValue() is missing this special casing of kPreferMailFormatProperty.
Comment 13 :aceman 2013-06-10 14:22:06 PDT
Created attachment 760591 [details] [diff] [review]
patch v2

OK, seems to work.
Comment 14 Mark Banner (:standard8) 2013-06-24 10:37:20 PDT
Comment on attachment 760591 [details] [diff] [review]
patch v2

Review of attachment 760591 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great.
Comment 15 Mark Banner (:standard8) 2013-06-24 11:41:01 PDT
https://hg.mozilla.org/comm-central/rev/cfb9aed1790d

Note You need to log in before you can comment on or make changes to this bug.