The default bug view has changed. See this FAQ.

Contacts API: Improve number lookup

RESOLVED FIXED in mozilla14

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

4.79 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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 +1-234-56789 should be found with

234-567
1234
34567
(Assignee)

Comment 1

5 years ago
So with this patch we can lookup:
+1-234-567
Assignee: nobody → anygregor
(Assignee)

Comment 2

5 years ago
Created attachment 607644 [details] [diff] [review]
patch

We still can't find +1-234-567 with 234-567. Do we need this?
Attachment #607644 - Flags: review?(bent.mozilla)
(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.
(Assignee)

Comment 4

5 years ago
(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
+1-234-567-890 and you look for 234567-890
(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
> +1-234-567-890 and you look for 234567-890

Ah. Yeah that probably doesn't need to work.
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.
Attachment #607644 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd34ce76aed2
Awesome, it also fixes the small "find contact from phone number" regression we had in the Gaia dialer app.
https://hg.mozilla.org/mozilla-central/rev/bd34ce76aed2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.