737456 - Contacts API: Improve number lookup

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Gregor Wagner [:gwagner]]

Looking up contacts with numbers needs some improvements.
Currently we only support a substring lookup from the beginning.

For numbers we need more.
For example [phone] should be found with

234-567
1234
34567

Comment 1

[Gregor Wagner [:gwagner]]

So with this patch we can lookup:
+1-234-567

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

We still can't find +1-234-567 with 234-567. Do we need this?

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to Gregor Wagner [:gwagner] from comment #2)
> We still can't find +1-234-567 with 234-567. Do we need this?

I would say we do.

Comment 4

[Gregor Wagner [:gwagner]]

(In reply to Philipp von Weitershausen [:philikon] from comment #3)
> (In reply to Gregor Wagner [:gwagner] from comment #2)
> > We still can't find +1-234-567 with 234-567. Do we need this?
> 
> I would say we do.

actually this example works...
the thing that doesn't work is a combination like
[phone] and you look for 234567-890

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to Gregor Wagner [:gwagner] from comment #4)
> (In reply to Philipp von Weitershausen [:philikon] from comment #3)
> > (In reply to Gregor Wagner [:gwagner] from comment #2)
> > > We still can't find +1-234-567 with 234-567. Do we need this?
> > 
> > I would say we do.
> 
> actually this example works...
> the thing that doesn't work is a combination like
> [phone] and you look for 234567-890

Ah. Yeah that probably doesn't need to work.

Comment 6

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

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

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

::: dom/contacts/fallback/ContactDB.jsm
@@ +226,5 @@
>        if (aContact.properties[field] && contact.search[field]) {
>          for (let i = 0; i <= aContact.properties[field].length; i++) {
> +          if (aContact.properties[field][i]) {
> +            if (field == "tel") {
> +              //Special case telephone number. 

Nit: Here and below add a space between // and first letter.

@@ +229,5 @@
> +            if (field == "tel") {
> +              //Special case telephone number. 
> +              // "+1-234-567" should also be found with 1234, 234-56, 23456
> +
> +              //Store "+1-234" as ["+1-234", "1-234", "-234"...]

Nit: maybe say that you're chopping a character from the front each time.

@@ +231,5 @@
> +              // "+1-234-567" should also be found with 1234, 234-56, 23456
> +
> +              //Store "+1-234" as ["+1-234", "1-234", "-234"...]
> +              let number = aContact.properties[field][i];
> +              for(let i=0; i<number.length; i++) {

Nit: spaces around = and <

@@ +235,5 @@
> +              for(let i=0; i<number.length; i++) {
> +                contact.search[field].push(number.substring(i, number.length));
> +              }
> +              // Store +1-234-567 as ["1234567", "234567"...]
> +              let tmp = aContact.properties[field][i].match(/\d/g);

Nit: Maybe rename tmp to digits? And you can do:

  let digits = number;

@@ +238,5 @@
> +              // Store +1-234-567 as ["1234567", "234567"...]
> +              let tmp = aContact.properties[field][i].match(/\d/g);
> +              if (tmp && number.length != tmp.length) {
> +                tmp = tmp.join('');
> +                for(let i=0; i<tmp.length; i++) {

Nit: spaces here too.

Comment 7

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd34ce76aed2

Comment 8

[Etienne Segonzac (:etienne)]

Awesome, it also fixes the small "find contact from phone number" regression we had in the Gaia dialer app.

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/bd34ce76aed2