Closed Bug 949537 Opened 10 years ago Closed 10 years ago

contacts stored are not recognized without international prefix +569

Categories

(Core Graveyard :: DOM: Contacts, defect)

x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
1.4 S4 (28mar)
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: brg, Assigned: reuben)

Details

(Whiteboard: [ucid: comms49, 2.0, FT:COMMS] khepera_51296, [systemsfe])

Attachments

(2 files, 4 obsolete files)

Reported when using V1-train during IOT process.

1.- Turn on DuT with Movistar Chile Sim card.
2.- Store a contact at DuT with international prefix +569 and name (E.I: TEST: +56912345678)
3.- Then, arrange a call to stored number without international prefix +569, (E.I: 12345678)
4.- Check if the contact is recognized 

Bad behaviour:
Currently device does not show to user the contact name stored if it is dialed without international prefix +569

Expected behaviour:
For Chile: 
Make sure that contact is recognized with +569 omitted for mobile destination and +562 omitted for fixed destination

Reviewing severity now with the reporter to see when the fix could be needed.
Whiteboard: khepera_51296
Joe, can someone from comms check this? I think it might be customisation somewhere IIRC.
Flags: needinfo?(jcheng)
Hi guys! I cannot add much here but as far as my understanding there is not enough "intelligence" in the SimplePhoneMatcher object to deal with all the complexity of the Chilean phone numbering system which is kind of complex as I can see here: http://en.wikipedia.org/wiki/Telephone_numbers_in_Chile  (taking the Wikipedia for granted of course :-p ). As far as I can see, we just consider "56" as the international prefix of Chile as you can see at https://github.com/mozilla-b2g/gaia/blob/v1-train/shared/js/simple_phone_matcher.js#L257 and consequently only these type of simple matchings would be detected:
SOMEDIGITS <-> +56SOMEDIGITS <-> 00SOMEDIGITS or 011SOMEDIGITS (depending on the operator's MCC)

Anyhow, including Etienne so he can also comment on this.
Flags: needinfo?(etienne)
Sorry, there was a typo in my previous comment:
SOMEDIGITS <-> +56SOMEDIGITS <-> 0056SOMEDIGITS or 01156SOMEDIGITS (depending on the operator's MCC) (I forgot the prefix (56) in the 00 or 011 cases)
Thanks a lot Germán!

Chile is puhsing strongly to have this issue solved. Currently is a blocker for certification. 

Nominated to leo?
blocking-b2g: --- → leo?
(In reply to David Palomino [:dpv] from comment #4)
> Thanks a lot Germán!
> 
> Chile is puhsing strongly to have this issue solved. Currently is a blocker
> for certification. 
> 
> Nominated to leo?

Hi David -

So now both +56 and +569 are the valid international prefix for Chile? Because I cannot find official information stating that +569 is now also a valid international prefix for Chile..

Thanks
Flags: needinfo?(liuyongming)
Flags: needinfo?(Chenguoqiang)
Flags: needinfo?(liuyongming)
Flags: needinfo?(Chenguoqiang)
Hi David -

Sorry we are no expert for the Chile phone numbering system, so we think we better use a concrete example to capture the requirement.


So suppose I live in Chile, and in my mobile phone I have the following two contacts:

1. Home: +5628765432 (here +56 is Chile international code, 2 is the special code for landline, and 8765432 is the actual 7 digit landline number of Home)

2. Peter: +56998765432(here +56 is Chile international code, 9 is the special code for Mobile phone, and 98765432 is the actual 8 digit mobile phone number of Peter)

So now when I get a call from Peter, although the number is only "98765432", in the Incoming Call Notification I shall see the name "Peter" display

And when I get a call from my home, although the number is only "8765432", in the Incoming Call Notification I shall see the name "Home" display

Finally, I try to call Home directly by typing 022 8765432 on Dialer, in the Calling notification, I shall see "Home" display

Is the above statement correct?

Also if we get a call from landline(assume number is 1234567), and this number is not stored in the contact, will we see 7 digit number on the Incoming Call Notification(21234567), or just 1234567?

Sorry again about the long question

Thanks
Flags: needinfo?(dpv)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #6)
> Hi David -
Hi Vance, thanks for your help! (see replies inline)

> 
> Sorry we are no expert for the Chile phone numbering system, so we think we
> better use a concrete example to capture the requirement.
> 
> 
> So suppose I live in Chile, and in my mobile phone I have the following two
> contacts:
> 
> 1. Home: +5628765432 (here +56 is Chile international code, 2 is the special
> code for landline, and 8765432 is the actual 7 digit landline number of Home)
> 
> 2. Peter: +56998765432(here +56 is Chile international code, 9 is the
> special code for Mobile phone, and 98765432 is the actual 8 digit mobile
> phone number of Peter)
> 
> So now when I get a call from Peter, although the number is only "98765432",
> in the Incoming Call Notification I shall see the name "Peter" display
> 
> And when I get a call from my home, although the number is only "8765432",
> in the Incoming Call Notification I shall see the name "Home" display
> 
> Finally, I try to call Home directly by typing 022 8765432 on Dialer, in the
> Calling notification, I shall see "Home" display
> 
> Is the above statement correct?

As far as I know, yes, it's that correct. 

> 
> Also if we get a call from landline(assume number is 1234567), and this
> number is not stored in the contact, will we see 7 digit number on the
> Incoming Call Notification(21234567), or just 1234567?
> 

Honestly, I do not know. Let me check this with our colleagues in Chile (not clearing my ni yet)

> Sorry again about the long question
> 
> Thanks
(In reply to gtorodelvalle from comment #2)
> Hi guys! I cannot add much here but as far as my understanding there is not
> enough "intelligence" in the SimplePhoneMatcher object to deal with all the
> complexity of the Chilean phone numbering system which is kind of complex as
> I can see here: http://en.wikipedia.org/wiki/Telephone_numbers_in_Chile 
> (taking the Wikipedia for granted of course :-p ). As far as I can see, we
> just consider "56" as the international prefix of Chile as you can see at
> https://github.com/mozilla-b2g/gaia/blob/v1-train/shared/js/
> simple_phone_matcher.js#L257 and consequently only these type of simple
> matchings would be detected:
> SOMEDIGITS <-> +56SOMEDIGITS <-> 00SOMEDIGITS or 011SOMEDIGITS (depending on
> the operator's MCC)
> 
> Anyhow, including Etienne so he can also comment on this.

I think at some point we plan to stop using the SimplePhoneMatcher :)
But in the meantime, from what I understand of this specific issue, using the area code support to consider that Chile has an area code of length 1 should do the trick.

Something like
```
   _areaCodeSwipe: {
     '55': 2,
+    '56': 1,
     '44': 3,
     '1': 3
   },
```
Flags: needinfo?(etienne)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #6)
> Also if we get a call from landline(assume number is 1234567), and this
> number is not stored in the contact, will we see 7 digit number on the
> Incoming Call Notification(21234567), or just 1234567?
> 

Hi Vance, 

We'll see 21234567 (including the first "2"). Our colleagues there have confirmed this. 

Thanks!
David
Flags: needinfo?(dpv)
Flags: needinfo?(jcheng)
Hi, 

Is there any update on this issue? This is quite important for Chile, and I think it should be fixed for 1.1 and later versions... 

Thanks!
David
(In reply to David Palomino [:dpalomino] from comment #10)
> Hi, 
> 
> Is there any update on this issue? This is quite important for Chile, and I
> think it should be fixed for 1.1 and later versions... 
> 
> Thanks!
> David

Our partner did fix this issue, however, the solution they use is just a temp solution. I am thinking Mozilla should in charge of modifying this mechanism such that for all the other partners won't encounter this problem again while doing IOT in Chile

Dear Beatriz, what do you think?
Flags: needinfo?(brg)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #11)
> Our partner did fix this issue, however, the solution they use is just a
> temp solution. I am thinking Mozilla should in charge of modifying this
> mechanism such that for all the other partners won't encounter this problem
> again while doing IOT in Chile
> 
> Dear Beatriz, what do you think?
+1, Nominating to 1.4 as this will be a IOT blocker in Chile.
blocking-b2g: leo? → 1.4?
Flags: needinfo?(brg)
ni David.

Hi David, as Beatriz and David point out, this issue will be a blocker for Chile IOT, in 1.1 our partner use a temp solution to fix this one, could you help to check if we can have a formal/better modification for this issue in 1.4+ ?

Thanks

Vance
Flags: needinfo?(dscravaglieri)
will be part of 1.4? triage
Flags: needinfo?(dscravaglieri)
Component: Gaia::Dialer → DOM: Contacts
Product: Firefox OS → Core
triage: 1.4+ to avoid certification problems later
blocking-b2g: 1.4? → 1.4+
Is this the change from june 2013 or has this rule been around for a long time?

http://www.itu.int/dms_pub/itu-t/oth/02/02/T020200002A0001PDFE.pdf
The current info that we are using in phonenumberJS is
      <mobile>
        <nationalNumberPattern>9[5-9]\d{7}</nationalNumberPattern>
        <possibleNumberPattern>\d{8,9}</possibleNumberPattern>
        <exampleNumber>961234567</exampleNumber>
      </mobile>

So we assume that the 9 is part of the mobile number. And so does android afaik.
Hi Gregor!

Sorry for my lack of knowledge, we are using phonenumberjs right?

And as I see on the makefile we are getting the metadata directly from google's repo:
http://libphonenumber.googlecode.com/svn/trunk/resources/PhoneNumberMetadata.xml

Should that list include the country code for 569 as well?
D'oh!

Please forget my last comment, now I got it, and also agree's with Gregor, taking into account current pattern, 9 will be part of the number.
Perhaps we should have in phonenumberjs:

<mobile>
        <nationalNumberPattern>9?[5-9]\d{7}</nationalNumberPattern>
        <possibleNumberPattern>\d{8,9}</possibleNumberPattern>
        <exampleNumber>961234567</exampleNumber>
</mobile>
Hi Francisco -

TCL Kunny is willing to contribute his patches for this issue. Maybe it is not perfect or flawless, but it do pass the Chile IOT test. Hope the patches can help you on this issue~

Hi Kunny, thanks for the patch!
Flags: needinfo?(francisco.jordano)
Not the expert here, but could we perform those tricks instead on the phonenumberjs library, like we have for Brazil already:

https://github.com/andreasgal/PhoneNumber.js/blob/master/xml2meta.py#L82-L83

The patch LGTM, although, I would prefer to perform that validation via the metadata library, also removing the debug option will be necessary.

Anyway, here Gregor (that has the knowledge about the matter) should decide.

Cheers,
F.
Flags: needinfo?(francisco.jordano) → needinfo?(anygregor)
Attached patch patch (obsolete) — Splinter Review
Can you try this patch?
Flags: needinfo?(anygregor)
Whiteboard: khepera_51296 → khepera_51296, [systemsfe]
(In reply to Gregor Wagner [:gwagner] from comment #24)
> Created attachment 8386309 [details] [diff] [review]
> patch
> 
> Can you try this patch?

Hei Gregor, unfortunately I cannot test the patch (just realized when I got it compiled ;)), cause I don't own a Chilean SIM :(

Does it take into account, both the mobile and land line requirements?
Hi Gregor,

In fact, local support would be needed to test a new patch and checked if all the scenarios are successfu, is there any concern with the patch provided by Kunny Liu?. In this case we already know that the certification process in chile is successfuly
(In reply to Noemí Freire (:noemi) from comment #26)
> Hi Gregor,
> 
> In fact, local support would be needed to test a new patch and checked if
> all the scenarios are successfu, is there any concern with the patch
> provided by Kunny Liu?. In this case we already know that the certification
> process in chile is successfuly

sorry, the comment was updated before finishing it:

Local support would be needed to test a new patch and check if all the scenarios are successfully covered. Is there any concern with the patch provided by Kunny Liu?. In this case we already know that the certification process in chile is guaranteed. Thanks
Flags: needinfo?(anygregor)
(In reply to Noemí Freire (:noemi) from comment #27)
> (In reply to Noemí Freire (:noemi) from comment #26)
> > Hi Gregor,
> > 
> > In fact, local support would be needed to test a new patch and checked if
> > all the scenarios are successfu, is there any concern with the patch
> > provided by Kunny Liu?. In this case we already know that the certification
> > process in chile is successfuly
> 
> sorry, the comment was updated before finishing it:
> 
> Local support would be needed to test a new patch and check if all the
> scenarios are successfully covered. Is there any concern with the patch
> provided by Kunny Liu?. In this case we already know that the certification
> process in chile is guaranteed. Thanks

We don't want the patch provided in our code base. We should fix it in a general way and not start special-casing countries.
Flags: needinfo?(anygregor)
PM triage. Keep this a 1.4.
Attached patch tests (obsolete) — Splinter Review
We have code in place that solves this problem for other countries in south america and we can fix it with a one liner: https://bug949537.bugzilla.mozilla.org/attachment.cgi?id=8386309
I also created the tests for this. Can we test this patch in a real scenario?
Flags: needinfo?(noef)
Assignee: nobody → anygregor
Attached patch tests (obsolete) — Splinter Review
Attachment #8387925 - Attachment is obsolete: true
Attachment #8387928 - Flags: review?(reuben.bmo)
Attachment #8386309 - Flags: review?(francisco.jordano)
Gregor, as I explained already in the past (bug 972457), I think our substring matching code is broken right now, and therefore we should rather fix the "NationalMathingFormat" property returned by PhoneNumber.js for Chile...

That said, this case will likely help us find out if the substring matching code is really broken. :/
Unfortunately don't have the hardware to test this :( (don't have a SIM card), and testing in Chile requires to expend some money, Noemi is looking to see if we can spend some resources on it.

Also we are trying to flash a custom build to trick the mnc and mcc to force Chile information and try to match.

Don't know if it's the best solution, as Julien commented I tried to modify the PhoneNumber.js, did modify the metadata for the mobilenumber, but didn't make it to the PhoneNumberMetadata :( (unfortunately don't know the way PhoneNumberLib works)
Comment on attachment 8387928 [details] [diff] [review]
tests

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

::: dom/contacts/tests/test_contacts_international.html
@@ +53,5 @@
>  };
>  
> +var number4 = {
> +  local: "98765432",
> +  international: "+5698765432"

I don't think this is testing the right thing, the test passes without the patch. The international number is supposed to be country code + 9 + local number (8 digits), right?
So something like:

        local:       87654321
international: +56 9 87654321

@@ +290,5 @@
> +      ok(findResult1.id == sample_id1, "Same ID");
> +      next();
> +    };
> +    req.onerror = onFailure;
> +  },

Please add a test for matching with a number someone in Chile would use to call a landline, like "0227654321". Can someone who actually knows about how to call phones in Chile confirm this? Wikipedia[0] doesn't cite sources.

[0] https://en.wikipedia.org/wiki/Telephone_numbers_in_Chile#Mobile_phone_numbers

::: dom/contacts/tests/test_contacts_substringmatchingCL.html
@@ +65,5 @@
> +    };
> +    req.onerror = onFailure;
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");

"Retrieving by last 8 digits"

@@ +66,5 @@
> +    req.onerror = onFailure;
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");
> +    var length = prop.tel[0].value.length;

This isn't used.

@@ +67,5 @@
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");
> +    var length = prop.tel[0].value.length;
> +    var num = "61234567"

Make it |prop.tel[0].value.slice(-8);| or something like that, instead of hardcoding it.

@@ +75,5 @@
> +    req = mozContacts.find(options);
> +    req.onsuccess = function () {
> +      is(req.result.length, 1, "Found exactly 1 contact.");
> +      findResult1 = req.result[0];
> +      ok(findResult1.id == sample_id1, "Same ID");

ise(findResult1.id, sample_id1, "Same ID");

@@ +82,5 @@
> +    };
> +    req.onerror = onFailure;
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");

"Retrieving by last 9 digits"

@@ +84,5 @@
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");
> +    var length = prop.tel[0].value.length;
> +    var num = "961234567"

Ditto here for length and hardcoding.

@@ +92,5 @@
> +    req = mozContacts.find(options);
> +    req.onsuccess = function () {
> +      is(req.result.length, 1, "Found exactly 1 contact.");
> +      findResult1 = req.result[0];
> +      ok(findResult1.id == sample_id1, "Same ID");

ise(findResult1.id, sample_id1, "Same ID");

@@ +99,5 @@
> +    };
> +    req.onerror = onFailure;
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");

"Retrieving by last 6 digits"

@@ +101,5 @@
> +  },
> +  function () {
> +    ok(true, "Retrieving by substring 1");
> +    var length = prop.tel[0].value.length;
> +    var num = "234567"

Ditto here for length and hardcoding.
Attachment #8387928 - Flags: review?(reuben.bmo)
Also, it looks like Android's PhoneNumberMetadata.xml already handles the new Chilean numbers, so why are we doing substring matching instead of using nationalMatchingFormat like Julien suggested?
Target Milestone: --- → 1.4 S3 (14mar)
Just giving some feedback, still trying to figure out if we can try this patch in Chile :(
Comment on attachment 8386309 [details] [diff] [review]
patch

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

Taking into account Reuben comments, not sure if this patch solves completely the problem.

As well, don't have all the info here on how the PhoneNumberMetadata.xml is used, as Reuben comments, nationalNumberPattern contains the regexp to deal with new format, but don't know if the nationalNumberPattern WITHIN the mobile section it's used.
Attachment #8386309 - Flags: review?(francisco.jordano) → review-
(In reply to Gregor Wagner [:gwagner] from comment #31)
> We have code in place that solves this problem for other countries in south
> america and we can fix it with a one liner:
> https://bug949537.bugzilla.mozilla.org/attachment.cgi?id=8386309
> I also created the tests for this. Can we test this patch in a real scenario?

Hi Gregor,

As commented by Francisco, trying to figure out how to test the patch locally once ready. Thanks!
Flags: needinfo?(noef)
PM triage this bug.  It should stay in 1.4
Assignee: anygregor → reuben.bmo
So the problem here is that the mobile calling code (9) is included in the national number. Ideally we'd have a property on the parsed number that is the "minimum dial-able number" or something like that, without the leading digits. For "+56 9 747xxxxx" that would be "747xxxxx". Unfortunately libphonenumber (and consequently PhoneNumber.js) has no such concept. Considering this bug is a blocker and substring matching works, I think we should just go ahead and configure it for Chile.
Attachment #8385955 - Attachment is obsolete: true
Attachment #8386309 - Attachment is obsolete: true
Attachment #8387928 - Attachment is obsolete: true
Attachment #8391642 - Flags: review?(anygregor)
Mostly needs a rubberstamp. I updated the comment in b2g.js because it uses area code 11 (Sao Paulo), which is now using 9 digit mobile numbers.

PS: This means we probably have a bug with |filterOp:'match'| for Brazilian numbers: if you have contact A with number "31 8765 4321" and someone from SP calls you, from number "11 98765 4321", we'll show contact A in the dialer. That's one of the cases where substring matching is not enough. I'll check this and file a bug.
Attachment #8391645 - Flags: review?(anygregor)
Comment on attachment 8391642 [details] [diff] [review]
Setup substring matching for Chile, and add some tests

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

::: dom/contacts/tests/test_contacts_international.html
@@ +291,5 @@
> +      ise(findResult1.id, sample_id1, "Same ID");
> +      next();
> +    };
> +    req.onerror = onFailure;
> +  },

Lets remove the local number from number4 and check that the lookup with the local number won't give us a result if susbstringmatching for CL is not enabled. So if the metadata for phonenumberJS changes we can detect it here.

::: dom/contacts/tests/test_contacts_substringmatchingCL.html
@@ +132,5 @@
> +      next();
> +    };
> +    req.onerror = onFailure;
> +  },
> +  

nit: whitespace
Attachment #8391642 - Flags: review?(anygregor) → review+
Comment on attachment 8391645 [details] [diff] [review]
Some minor cleanup of substring matching tests

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

Thanks!
Attachment #8391645 - Flags: review?(anygregor) → review+
Target Milestone: 1.4 S3 (14mar) → 1.4 S4 (28mar)
I wonder if we're running that test with some random country's substring matching enabled :/
https://hg.mozilla.org/mozilla-central/rev/650680bb6516
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Oops, dealing with the test failure made me forget about the other patch. This one doesn't have to be uplifted, is there a Bugzilla protocol for signaling that in the attachment?

https://hg.mozilla.org/integration/b2g-inbound/rev/3e0138968c61
Hi, 

This issue has been tested locally by Pablo, one of our community members in Chile (thanks A LOT Pablo!!!). 

Pablo did some test with both mobile and fixed lines, mobile terminated and mobile originated, with 1.5 nightly build. In all combinations the behavior was ok. 

So moving this issue to VERIFIED. 

Again, thanks Pablo!!!
David
Status: RESOLVED → VERIFIED
Whiteboard: khepera_51296, [systemsfe] → [ucid: comms49, 2.0, FT:COMMS] khepera_51296, [systemsfe]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.