Gecko http://hg.mozilla.org/releases/mozilla-b2g18/rev/2d3a45c58878 Gaia 0bfcaf8546c6d365b0474d980a154d859d22c872 BuildID 20130529230207 Version 18.0 Getting a message from a contact with multiple phone numbers and types, the wrong number and type is shown in the message pane. In my case I sent the message from my O2 phone but the number shown is my land line. In my contact information this is the first listed number. It looks like a regression from the latest SMS change. Probably from bug 876774?

It looks like this only involves threading or the display of the number. When I reply the message get sent to the right phone number.

:whimboo, can you please provide a couple of screenshots to illustrate the description ? In addition already cc'ing Alexandre to check if the problem is understood and a regression from 876774 as suspected ?

I will attach two screenshots. One for the contact entry in the database and one for the sms application and how the phone number is shown. It should help you to understand the problem in not being able to find the right phone number.

Ups, sorry. I mixed up the bugs. The screenshots were not for this one.

The following two screenshots will show the exact problem. The message was sent from the '+49(163)...' number but is displayed as being sent via my office phone.

I can't reproduce: when I create a contact with two distinct phone numbers and that I send a message to each one, I get two entries in the thread list, and each one is mapped on the correct phone number. My gaia is at d5d898bb28b5ff931ec2bbdfe0b096b8e45329c9 ...

I think you misunderstood the steps. So I will make them clearer: 1. Create a contact with two phone numbers. The first should be a landline you never will be able to send a sms from. The second number setup with a number of your second phone. 2. Send a message from your second phone to your Firefox OS phone 3. Open the SMS application and the new thread 4. Observe that the phone number lists the landline and not the other mobile phone

I just redid this, and I still can't reproduce: the thread shows the correct mobile phone number and not the landline number.

Henrik, I cant reproduce the bug as well... :S

Can you both make screenshots please how your contact entries look like? Not sure if it is important to have +() and spaces in the numbers.

I've retried with a contact which has sent a SMS from +336xxxxxxxx: contact has two phones numbers, first one tagged as home and being +33(2)47xxxxxx, the second one being the correct +336xxxxxxxx number. With this setup, thread list shows the contact name and the number and type displayed when reading the thread are correct, i.e., it's not the landline number but the mobile number. I can't type a phone number with spaces. If you can check in your case that removing the spaces worksaround the bug, it would be very cool.

Adding qawanted - we should try to see if someone can reproduce this issue since this could be a blocker if it is reproducible - discussed during 6-4 triage.

I'm currently in MV, so if any of the B2G folks are around and want to have a look at my phone, please let me know.

Tried to repro this issue on: Unagi Build ID: 20130529230207 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/2d3a45c58878 Gaia: 0bfcaf8546c6d365b0474d980a154d859d22c872 The only issue I could see was having the number that was called not showing the contacts name, it only showed as the number(when put as the second phone number for the contact).

Tested on: Unagi Build ID: 20130607111630 Gecko-7af1737.Gaia-b5937ae I can't reproduce the bug. The thread view shows the contact name and the correct mobile phone number (if there isn't carrier register), and the name and the carrier if this one is register. See attachments: "screenshot(contact entry)I", "screenshot (thread)II_1", "screenshot (thread)II_2"

Guys, this is not how it should work. As I have mentioned it above I can perfectly see this on my phone, and I asked if someone in MV can have a look at. I'm still here today, so take your chance. I showed it to Marcia yesterday, so she can confirm that behavior.

Henrik please check in with Tony who will be in the MV office today.

I talked with Fabrice and he was able to see that behavior. By our talk I changed the first phone number of my own contact which is the work number displayed in the message, and it gets dynamically updated. That means we really grab the number dynamically from the contacts, and take the first one instead the one the message really came from.

Ok, one good news is that I know now why it is happening. The mobile number I have sent the message from is in the format of +xx(xyz)12345678. When I remove the brackets in the address book the number is finally shown as the one for the sender. So this issue depends on the correct detection of phone numbers with brackets included. Not sure if there is a bug about it, if you know please add the dependency.

I think that's enough qawanted here. The underlying problem is clear and simply has to be fixed. The good thing is that this only happens for hte display, but not when responding to such a message. That will be send to the right phone.

Blocking since this is our data sanitation at issue (checking for brackets) and it impacts who the user thinks they are messaging.

(In reply to Henrik Skupin (:whimboo) from comment #24) > When I > remove the brackets in the address book the number is finally shown as the > one for the sender. > Could it be that doing that change effectively moves the number on top of the internal stack, and as a result it's been displayed ? Anyway, I'm taking this. Henrik, thanks for this report, I agree with you that if you're confident that you have the problem we should not close as "works for me"...

Gregor, I think we need help from Gecko here, because we don't have access to the PhoneNumber library. I can see 2 things that could help from the mozContacts API: * get a normalized value in addition to the user-entered value (eg in a "normalizedValue" property), so that we can easily compare with the numbers we get from the network * when searching a contact with a "tel" search, have an option to return only the matched number for this contact, instead of returning all numbers for the found contact. The first one looks like a need-to-have to properly support some use cases, the second one is more a nice-to-have and would allow us to not do this filtering in Gaia, which feels cleaner. Should I file one or 2 bugs about these cases ?

About the second proposition: I'd be happy with another method, or getting that info with the usual results in another property too, ie without needing an option.

Filed bug 883923 and bug 883929, any of them would help us here.

(In reply to Julien Wajsberg [:julienw] from comment #28) > Gregor, I think we need help from Gecko here, because we don't have access > to the PhoneNumber library. > > I can see 2 things that could help from the mozContacts API: > > * get a normalized value in addition to the user-entered value (eg in a > "normalizedValue" property), so that we can easily compare with the numbers > we get from the network I don't think exposing the normalized number is a good idea. Maybe it's simpler to just expose the normalization interface? > * when searching a contact with a "tel" search, have an option to return > only the matched number for this contact, instead of returning all numbers > for the found contact. Currently we can't return partial contacts. If you save a partial contact, you would overwrite the existing one and wipe out all the missing fields.

(In reply to Gregor Wagner [:gwagner] from comment #31) > > Currently we can't return partial contacts. If you save a partial contact, > you would overwrite the existing one and wipe out all the missing fields. I understand this. Please check bug 883929 as I've put other possible suggestions here.

Just having a discussion with Julien and Gregor, we're hoping to normalize the sender/receiver of the messages exposing to the Gaia end, so that the (normalized) numbers can be correctly associated with the (normalized) numbers in the Contacts API. Fire bug 886723.

(In reply to Gene Lian [:gene] from comment #33) > Just having a discussion with Julien and Gregor, we're hoping to normalize > the sender/receiver of the messages exposing to the Gaia end, so that the > (normalized) numbers can be correctly associated with the (normalized) > numbers in the Contacts API. > > Fire bug 886723. Sorry, I've closed bug 886723 as WONTFIX. Anyone interested in this method may want to drop a comment there.

Michal will work on this bug as soon as bug 883923 is fixed. We'll try to do without bug 886723.

Julien, I think this was actually fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=881248

not in all cases :) (eg: the "number with letters" case). I think we'll remove part of that PR (eg the removeNonDialables part) and some of the tests, as I was explaining in https://github.com/mozilla-b2g/gaia/pull/10472#issuecomment-19687960 ... at the end we'll get the normalized value out of the mozcontacts api.

So, we're still waiting for bug 883923 here...

unassigning myself as I won't be available in the next 2 weeks. Should not be difficult once bug 883923 is solved.

In MozTrap added SMS Suite Test Case #9010 - Verify the number displayed in the message pane is the phone number and type the message originated from

Over to David to reassign. Thanks for the heads up Julien.

Could you please flag me for review and rwaldron for feedback on any patch here? Thanks!

Etienne - can you help with reassignment?

Etienne, I could be in charge of this. Wdyt? Re-assign to me if you agree! :)

(In reply to Borja Salguero [:borjasalguero] from comment #44) > Etienne, I could be in charge of this. Wdyt? Re-assign to me if you agree! :) It's SMS, you know better than me :)

Waiting Gecko's patch for moving forward here!

Since I'm back from holiday feel free to reassign or ask review from me here (as I followed the whole story from the start).

I'll definitely ping you on monday about this ;)

:fcampo, :julienw : what are the next steps here ? Trying to get an eta given how close we are to 1.1 release.

Well, we're still waiting for bug 883923.... that bug is moving forward but they had to change the whole approach after a first patch was quite finalized.

However this is imho not a so big bug to hold the release from, so feel free to move to 1.2.

asking leo? again, I'm not sure this should block the release. Henrik, could you please update us of what is the current status on 1.1 for this bug ? My understanding is that we fixed some of the cases in bug 881248 (doing a limited fuzzy matching in Gaia) and that the current remaining bug happens only with what I call "numbers-with-letters" (eg: 1-800-BUY-A-CAR). My second understanding of the code (I haven't checked this) is that when the bug happens we no longer display the first phone but rather we display nothing, although I'm not sure of that.

Making it a non-blocker for leo given this may have been mitigated by patch in bug 881248. Henrik, can yo please help with comment #52 ?

(In reply to Julien Wajsberg [:julienw] from comment #52) > Henrik, could you please update us of what is the current status on 1.1 for > this bug ? Julien, how can I enter white-spaces in the phone number on the Firefox OS device? I removed those during testing a while back, and as it seems like I cannot do it via the UI. Also I cannot re-import the contacts from Google and I don't want to reset my phone. Which database I would have to update?

Henrik> maybe you can create a new dummy contact on Google and import only this one ?

Bug 883923 was fixed so I guess we can start moving forward here.

(In reply to Julien Wajsberg [:julienw] from comment #55) > Henrik> maybe you can create a new dummy contact on Google and import only > this one ? As mentioned above, there is no way to reimport the contacts from Google when running the FTU app again.

I think you can import from the contacts app too.

I have re-imported my own contact from google contacts, deleted the old thread, and send a new message from my other mobile phone. This time the number is displayed correctly. So I assume we are fine here now.

Thanks Henrik ! So confirming for koi then :) Let's use the Fuzzy Matcher to back our Utils.compareDialables implementation !

I'm focused right now in new features for this sprint and bugfixing for the next one, so if someone thinks this should be done right now, feel free to reassign, as I won't be able to fix it soon

Thanks Rick

Rick, do you think you'd be able to finish this ? I don't think there is much to do here. We could as well move this to 1.3, but this bug is there for too long ;)

(In reply to Julien Wajsberg [:julienw] from comment #63) > Rick, do you think you'd be able to finish this ? I don't think there is > much to do here. > > We could as well move this to 1.3, but this bug is there for too long ;) I will work on this next (ie. today)

Waiting for response to https://bugzilla.mozilla.org/show_bug.cgi?id=883923#c86

The only remaining case is small let's move this to 1.3. If we have a fix soon we could request an approval.

Part 1: - Implement a desktop version of navigator.mozPhoneNumberService - Move all desktop-specific code into js/desktop-only/ - Updates startup.js to include new dir locations and mock files The following files are copied almost exactly from https://mxr.mozilla.org/mozilla-central/source/dom/phonenumberutils: - PhoneNumber.js - PhoneNumberNormalizer.js - PhoneNumberMetaData.js - mcc_iso3166_table.js Part 2: - Rename Utils.compareDialables => Utils.probablyMatches - Utils.probablyMatches now normalizes with navigator.mozPhoneNumberService.normalize - Update navigator.mozContacts.find desktop "mock" to also normalize when searching numbers

Instead of copying all of PhoneNumberService so that we can use it when running the app in Firefox, I'd rather find a way to enable it with a DEBUG=1 profile. Needinfo Kevin and Mike to see if they have an idea.

(In reply to Julien Wajsberg [:julienw] (in MozSummit until next monday) from comment #68) > Instead of copying all of PhoneNumberService so that we can use it when > running the app in Firefox, I'd rather find a way to enable it with a > DEBUG=1 profile. > > Needinfo Kevin and Mike to see if they have an idea. That would be the most ideal approach :)

It should certainly be possible to have all of this functionality inside of nightly without copying the code. We may have to have either a pref, or to import some jsm file. Contacts works by importing a jsm file in the addon. Unfortunately, my gecko skills are still lacking so I'm not able to quickly produce a patch for this. If you need this, I would say that you can go ahead and land as-is, and we can open a follow-up bug to use the platform code. In the meantime I'll also try to dig into gecko code to figure out the proper way of doing it.

Having PhoneNumberService on desktop (behind a pref) should be simple.

(In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday and friday) from comment #68) > Instead of copying all of PhoneNumberService so that we can use it when > running the app in Firefox, I'd rather find a way to enable it with a > DEBUG=1 profile. > > Needinfo Kevin and Mike to see if they have an idea. Yeah, should be super simple. We can just add a check for the pref here: https://mxr.mozilla.org/mozilla-central/source/dom/base/Navigator.cpp#1653

(In reply to Reuben Morais [:reuben] from comment #71) > Having PhoneNumberService on desktop (behind a pref) should be simple. Why do we need this? For testing?

Gregor> both for running in Firefox Desktop and for running unit tests.

Blocking as it is a regression

blocking per comment 75

The remaining bug here is not a regression, the regression was fixed in bug 881248.

I've outlined a new plan in https://github.com/mozilla-b2g/gaia/pull/12513#issuecomment-26582824 that would let us work without exposing mozPhoneNumberService to content with a pref (bug 926414).

Comment on attachment 813555 [details] https://github.com/mozilla-b2g/gaia/pull/12513 r=me with the nits fixed and provided Travis is green-ish

Hey Rick, I just noticed we haven't closed this yet ;) Any chance you can wrap up soon?

Just noticed it landed but this bug was not updated. master: d7696ab98db897db75a7fb633cbe3fc3a68ae820