861720 - Phone number to contact mapping matches if the number is a proper substring

Status: VERIFIED FIXED

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

Description

[Matthew N. [:MattN]]

STR:
1) Receive an SMS from a shortcode
* Easy wasy with t-mobile is to receive send an SMS to your phone using their email gateway: [e-mail]
2) Look at the shortcode that the SMS was from (e.g. 3769)
3) Add a contact which happens to contain this number as a substring (e.g. [phone]) and give it a name
4) Go back to the SMS message app

Expected result:
SMS app should continue to show the message is from 3769 (no contact mapping)

Actual result:
SMS app mistakenly matches the SMS from 3769 to the contact with number [phone]

I have run into this many times with a few hundred contacts and random shortcodes from t-mobile. It's misleading and inaccurate which could cause users to think they are receiving messages from someone other than the actual sender.

I am running v1.1 beta build 20130404070202

See also bug 857460 about ignoring formatting for phone number matches.

Comment 1

[Matthew N. [:MattN]]

I think the code path is:

apps/sms/js/thread_list_ui.js   updateThreadWithContact
> Contacts.findByString(number, function gotContact(contacts) {

apps/sms/js/contacts.js         Contacts.findByString
>       return this.findBy({
>         filterBy: ['tel', 'givenName', 'familyName'],
>         filterOp: 'contains',

Since we are using the "contains" operator here, [phone] is returned because it contains 3769. We should do additional checks that the number is a match by ignoring formatting (bug 857460) and then comparing the length or something similar.

I also don't think we should be searching the names from this caller (since it's extra work) so it seems like there should be another function named "findByNumber" which only searches the "tel" field and could do these additional checks.

Comment 2

[David Palomino [:dpalomino]]

Hi, 

I think this could have an important impact, specially with short numbers typically used by operators. 

I nominate it for tef?

Thanks!
David

Comment 3

[Andrew Overholt [:overholt]]

Etienne & Francisco, are dialer and SMS aligned in behaviour here?  What do you think of this bug?

Rob (I'm adding him on CC here) has offered UX input if required.  Set needinfo on him if you'd like his help.

Comment 4

[Etienne Segonzac (:etienne)]

What we do in the dialer is that if a number is less than 7 characters long (ie. a short number [1]) we request an "equals" match. So we don't have this issue.

[1] http://en.wikipedia.org/wiki/E.164

Comment 5

[Fernando Campo (:fcampo)]

I think that this was solved on bug 856991, so it's probably a matter of old build with the code not updated. Build is from 04/04 and the bug was solved on 25/04.

Could you please try with a recent build, or update your gaia code?

Comment 6

[Andrew Overholt [:overholt]]

Matt, can you try with a more recent build?

Comment 7

[Matthew N. [:MattN]]

The patch in bug 856991 only landed four days after I filed this bug. https://github.com/mozilla-b2g/gaia/commit/51931fee784c0a9e81fa5afc525007e8ad9f2340

I just received an updated beta v.1.1.0 build today and I can confirm that this is now fixed.

Comment 8

[Andrew Overholt [:overholt]]

(In reply to Matthew N. [:MattN] from comment #7)
> The patch in bug 856991 only landed four days after I filed this bug.
> https://github.com/mozilla-b2g/gaia/commit/
> 51931fee784c0a9e81fa5afc525007e8ad9f2340
> 
> I just received an updated beta v.1.1.0 build today and I can confirm that
> this is now fixed.

Thanks, Matt!

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train and v1.0.1.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train and v1.0.1, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 51931fee784c0a9e81fa5afc525007e8ad9f2340
  <RESOLVE MERGE CONFLICTS>
  git commit
  git checkout v1.0.1
  git cherry-pick -x $(git log -n1 v1-train --pretty=%H)

Comment 10

[Daniel Coloma:dcoloma]

Hey, this was already uplifted in bug 856991

Comment 11

[Jeni]

Verified fixed on 

Inari Build ID: 20130506070205
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/ce67220b877d
Gaia: 1e598d8842920d9e0b1743dc6fe9390bd5f6e2df

and on 

Unagi Build ID: 20130506070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/00c554abfc17
Gaia: 9377636cee5ac55b9f1d68f598afc7aadfbb2b00

When user enters a number that has some of the number attached to a name it will no longer automatically send to saved user.  It will instead send to number entered in To field.