815833 - [contacts] Add PhoneNumberJS to ContactsDB

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

Add the international number to the search field for contacts.
An API search with the international number finds now a contact that was entered with the local number.
We add the international number to the search field for telephone numbers of the contact.

Comment 2

[Gregor Wagner [:gwagner]]

Andrea, can you test this?
Please pull your contacts DB from the phone before you test this patch. Just in case the upgrade code is buggy and I have to rerun the upgrade code with your DB.
It's in data/local/indexedDB/chrome.

Comment 3

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 4

[Borja Salguero [:borjasalguero]]

[:gwagner] Only one comment in your patch. Take care because there are phone numbers that could be not internationalized! For example, in Spain we have the following number of Movistar: 2236. If you go to the library and you ask for the 'parsed' version of it, you are gonna retrieve null! So in this case instead of the following line:

dump("Warning: No international number found for " + number + "\n");

You should add the request with the phone number that you receive directly, something as:
contact.search[field].push(number.toString());

For create a generic way of doing it (in SMS App we kept a DB with the same problems as this one, and we fix it! ;) ), method 'parse: function(aNumber)' should return or the 'internationalNumber' (if it's available), or 'aNumber' directly, in order to work properly with phone numbers with a length less than 8 characters (http://en.wikipedia.org/wiki/E.164). Thanks!!!

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 685867 [details] [diff] [review]
patch

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

r=me with one comment:

::: dom/contacts/fallback/ContactDB.jsm
@@ +171,5 @@
> +                    debug("InternationalFormat: " + parsedNumber.internationalFormat);
> +                    debug("InternationalNumber: " + parsedNumber.internationalNumber);
> +                    debug("NationalNumber: " + parsedNumber.nationalNumber);
> +                    debug("NationalFormat: " + parsedNumber.nationalFormat);
> +                    cursor.value.search.tel.push(parsedNumber.internationalNumber);

This could be avoided if parsed number == existing number.

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: patch

upgrade code....

Comment 7

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/71ffd1a30c08

Comment 8

[Gregor Wagner [:gwagner]]

Followup: 
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f1365db9a35

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/71ffd1a30c08
https://hg.mozilla.org/mozilla-central/rev/9f1365db9a35

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/927dd0a37a65
https://hg.mozilla.org/releases/mozilla-aurora/rev/26070fd9985b

https://hg.mozilla.org/releases/mozilla-beta/rev/46925afbd175
https://hg.mozilla.org/releases/mozilla-beta/rev/13bb74c3367e

Comment 11

[Borja Salguero [:borjasalguero]]

https://bugzilla.mozilla.org/show_bug.cgi?id=813621#c11

Comment 13

[Borja Salguero [:borjasalguero]]

Copy & Paste the scenario:

- Store one Contact with 612123123
- Request to Contact API with the internationalized one:
      var options = {
        filterBy: ['tel'],
        filterOp: 'equals',
        filterValue: '[phone]'
      };

- Call to API:
window.navigator.mozContacts.find(options);

EXPECTED: Retrieve the contact

CURRENTLY: No contact retrieved for the internationalized version of the phone number

Comment 14

[Gregor Wagner [:gwagner]]

(In reply to Borja Salguero [:borjasalguero] from comment #13)
> Copy & Paste the scenario:
> 
> - Store one Contact with 612123123
> - Request to Contact API with the internationalized one:
>       var options = {
>         filterBy: ['tel'],
>         filterOp: 'equals',

Equals currently only checks the number you entered. 'contains' should work.

Comment 15

[Borja Salguero [:borjasalguero]]

Thanks! Closing again ;)