1477438 - add telephone to the tooltip in IM contacts list

Status: RESOLVED FIXED

(Chat Core :: XMPP, enhancement)

Description

[matevz.jekovec]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180704192850

Steps to reproduce:

In our company, we store internal phone numbers of the users to Active Directory. For chat, we set up an XMPP server with AD authentication and we are using thunderbird chat. It would be really useful, if the telephone number is also shown among other information in the tooltip when you move the mouse over the contact.


Actual results:

The current tooltip includes userName, fullName, nickname, title, organization, email, birthday, locality, and country.


Expected results:

The tooltip should also include the telephone number.

Comment 1

[Patrick Cloke [:clokep]]

I don't think this would be that hard to do, and it makes sense to include. It looks like all we would need to do is add the proper field to the list of displayed fields (kTooltipFields) [1].

Unfortunately, I don't know what that field is called! You can likely pull it out of the debug log if you don't know off hand. (Debug log is found via right clicking on the account in the account manager and clicking "Copy debug log" Note that this can have sensitive things in it, although passwords should be blanked out.)

[1] https://dxr.mozilla.org/comm-central/source/chat/protocols/xmpp/xmpp.jsm#1450-1453

Comment 2

[matevz.jekovec]

Hi Patrick.

Pasting the relevant snippet of the XMPP response. Notice the <TEL><NUMBER>+xxxxxxxxx</NUMBER></TEL> inside the <vCard>. Thanks for your effort!

<iq xmlns="jabber:client" xml:lang="en" to="[e-mail]/141576165508345907451890" from="[e-mail]" type="result" id="ffcac345-2e96-4b7e-a241-2d96b07df1b8">
 <vCard xmlns="vcard-temp">
  <N xmlns="vcard-temp">
   <GIVEN xmlns="vcard-temp">
    Gašper
   </GIVEN>
   <FAMILY xmlns="vcard-temp">
    Fele-Žorž
   </FAMILY>
   <MIDDLE xmlns="vcard-temp"/>
  </N>
  <FN xmlns="vcard-temp">
   Fele-Žorž, Gašper
  </FN>
  <TEL xmlns="vcard-temp">
   <NUMBER xmlns="vcard-temp">
    [phone]
   </NUMBER>
  </TEL>
  <EMAIL xmlns="vcard-temp">
   <USERID xmlns="vcard-temp">
    [e-mail]
   </USERID>
  </EMAIL>
  <URL xmlns="vcard-temp"/>
  <ORG xmlns="vcard-temp">
   <ORGNAME xmlns="vcard-temp">
    Fakulteta za računalništvo in informatiko
   </ORGNAME>
   <ORGUNIT xmlns="vcard-temp">
    Laboratorij za vseprisotne sisteme
   </ORGUNIT>
  </ORG>
  <NICKNAME xmlns="vcard-temp">
   gfele
  </NICKNAME>
  <TITLE xmlns="vcard-temp">
   asistent
  </TITLE>
  <DESC xmlns="vcard-temp"/>
  <ADR xmlns="vcard-temp">
   <REGION xmlns="vcard-temp"/>
   <LOCALITY xmlns="vcard-temp">
    Ljubljana
   </LOCALITY>
   <CTRY xmlns="vcard-temp">
    SI
   </CTRY>
   <PCODE xmlns="vcard-temp">
    1000
   </PCODE>
   <STREET xmlns="vcard-temp">
    Večna pot 113
   </STREET>
  </ADR>
 </vCard>
</iq>

Comment 3

[Patrick Cloke [:clokep]]

Attachment: Patch v1

This adds telephone numbers to the tooltip and adds some basic tests for parsing vCards. Unfortunately I don't seem to have any contacts that provide a telephone number.

Comment 4

[Florian Quèze [:florian]]

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

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

The code change looks good, the low readability of the test is frustrating; I'll rs it if you can convince me writing this test in a readable way is more effort than it's worth.

::: chat/protocols/xmpp/test/test_parseVCard.js
@@ +12,5 @@
> +  if (aText)
> +    node.addText(aText);
> +  aParent.addChild(node);
> +
> +  return aParent

eslint should catch the missing semi colon here.

@@ +18,5 @@
> +
> +/*
> + * Example vCard from XEP-0054 section 3.1, example 2.
> + */
> +function get_example() {

This function is unfortunately hard to read :-/. How much effort would it take to write the XML markup in a string, and parse it the using the same parser we use for incoming stanzas?

Comment 5

[Patrick Cloke [:clokep]]

Attachment: Patch v2

Not doing the parsing of the strings was an attempt to separate tests into better units, but you're right that starting with the raw strings is much clearer. This patch does that. I also remembered to run eslint this time. :)

The examples from XEP-0054 (https://xmpp.org/extensions/xep-0054.html) were slightly modified by adding xmlns='jabber:client'. Our XMPP parser doesn't like it if you don't include a namespace.

One weird thing about this is that it takes the last telephone number. I feel like it should be an array (or object) with additional metadata. (Note that this issue seems to also exist for email addresses, and perhaps other elements in the vCard.)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d3ed91f7a8e0
Add telephone number to the XMPP tooltip. r=florian