856991 - [Buri][SMS]If there is 10010 exist in the phone number of 11 digits from one r contact, the Send SMS will abnormal

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.19.055
 Firefox os  v1.0.1
 Mozilla build ID: 20130327153857
 
 +++ This bug was initially created as a clone of Bug #427629 +++
 
 Created an attachment (id=371953)
 issue picture
 
 [WuYing][6915]
 DEFECT DESCRIPTION:
   if there is 10010 exist in the phone number,then you sent a SMS to 10010, the receiver will show the contact which include 10010.
 
 REPRODUCING PROCEDURES:
 For Example
 1.Save Phone number 13482110010 to Contact list
 2.Enter the SMS
 3.Send SMS to 10010
 4.But this SMS will show 13482110010 Contact.----KO
 
 
 
 EXPECTED BEHAVIOUR:
 Input 10010 number in recipient,the SMS should send to 10010
 
 
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 V114
 USER IMPACT:
 Medium
 REPRODUCING RATE:
 5/5
 For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #427629 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[xiupinglong]

Attachment: issue picture

Comment 2

[xiupinglong]

Hello Borja Salguero,

It seems that the match rules has some mistake. The short number matchs the contact which contains the number.

Comment 3

[Borja Salguero [:borjasalguero]]

Hi. It's true because currently we are working with 'contains' following Gecko suggestions. We could ask to Gecko developer in order to know how to deal with it.

Comment 4

[Gregor Wagner [:gwagner]]

Yeah multiple people reported such problems. I am working on a solution now and we should fix it next week.

Comment 5

[Alex Keybl [:akeybl]]

We're worried fixing this will represent a lot of risky work. Can QA confirm that:

1) The text does end up getting sent to 10010
2) Subsequent texts in that thread are sent to 10010
3) The user is still able to send a text messages to 13482110010

If these user scenarios aren't impacted, this is not a blocker. Even still, this may not be a blocker (given risk involved and small user impact).

Comment 6

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

I'm a little bit at a loss how we'd test this on our end. 10010 appears to be very specific to China's Unicom system. 

Is this something someone with a compatible phone system can help with?

Comment 7

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Incidentally, as a blind guess the problem may be that we're attempting to match numbers w/o area code being necessary. 

On iOS I believe if I call 555-1212, it'll automatically realize I mean 408-555-1212 and display the matching contact appropriately. We may just not have the regex formed in a way that it's specific to area/country codes, particularly if it attempts to be non-specific to locale.

Comment 8

[Tony Chung [:tchung]]

(In reply to Geo Mealer [:geo] from comment #7)
> Incidentally, as a blind guess the problem may be that we're attempting to
> match numbers w/o area code being necessary. 
> 
> On iOS I believe if I call 555-1212, it'll automatically realize I mean
> 408-555-1212 and display the matching contact appropriately. We may just not
> have the regex formed in a way that it's specific to area/country codes,
> particularly if it attempts to be non-specific to locale.

Per Triage, this isn't only affecting 5 digit numbers.   This bug is really about entering in any amount of short numbers in the app, and sending the SMS would end up sending to another number that matched the numbers from your contacts (ie. send SMS to short number "609", but it ends up going to a saved contact with "888-333-9609".    

Removing qawanted, since i saw it reproduced by a few people in the triage room.  (both dcoloma and Emily)

Comment 9

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

This patch introduces findByEqualString that does an equals op against mozContacts and replaces all calls (except the one in the find contact screen) by this call. I've added test cases.

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 738431 [details] [diff] [review]
Patch

Did a quick check, nothing weird on the code, but prefer Fernando do rplus after rechecking.

Thanks Jan!!

Comment 11

[Rick Waldron [:rwaldron]]

Copied from the PR: 

This doesn't match any existing platform's behaviour; both iOS Contacts and Android People lookup phone numbers from the beginning.

Comment 12

[Rick Waldron [:rwaldron]]

Strike the previous comment, I misunderstood which Contacts lookup behaviour was being modified.

Comment 13

[Jan Jongboom [:janjongboom] (inactive)]

Created bug 863198 to keep track of refactoring the Contacts module in SMS.

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

Ping fernando.campo for re-r?.

Comment 15

[Fernando Campo (:fcampo)]

Comment on attachment 738431 [details] [diff] [review]
Patch

Thanks for the quick fix and the followup, patch looks nice, tested and works fine.

Comment 16

[Jan Jongboom [:janjongboom] (inactive)]

Landed in https://github.com/mozilla-b2g/gaia/commit/51931fee784c0a9e81fa5afc525007e8ad9f2340

Comment 17

[Julien Wajsberg [:julienw]]

Jan, I'm quite sure this won't apply cleanly on v1.0.1, would you please prepare a specific PR for 1.0.1 ?

Comment 18

[Julien Wajsberg [:julienw]]

Jan, we need you here, please ;)

Comment 19

[Jan Jongboom [:janjongboom] (inactive)]

Oops, didn't read this. So you need a PR against the v1.0.1 branch on Gaia?

Comment 20

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch for 1.0.1

Here's a patch that merges against v1.0.1

Comment 21

[Julien Wajsberg [:julienw]]

Comment on attachment 741772 [details] [diff] [review]
Patch for 1.0.1

stealing

Comment 22

[Julien Wajsberg [:julienw]]

Comment on attachment 741772 [details] [diff] [review]
Patch for 1.0.1

r=me

I'm sure we'll run into problems with internationalized/regional numbers too but well, let's fix this.

Comment 23

[Julien Wajsberg [:julienw]]

James, could you please uplift this ? I'm afraid to run into conflicts if we delay uplifting this too much.

Comment 24

[James Lal [:lightsofapollo]]

v1-train: b1e5cee77ff87bde14cfa6a4b04f12cb68078456
v1.0.1: 68508bf82925aa389eb4d45d5c9ed5f76b78d81d

Comment 25

[sarsenyev]

This issue doesn't reproduce on Inari and Leo devices

Environmental  Variables:
Unagi Build ID: 20130430070201
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/010498e599a7
Gaia: de0f1fc6c58616b8a33fca482f1f8684d4e74e9e

Environmental  Variables:
Unagi Build ID: 20130426070204
Kernel Date: Mar 15
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6c2493de1441
Gaia: c9046a7acef33328977840176ff5574720d2c74c

Comment 26

[Cody Roesch [:croesch]]

Unable to test on the Buri Device. Marking as QARegressExclude.