Closed Bug 1131938 Opened 7 years ago Closed 3 years ago

[Calllog][CallScreen] Not showing contact name if number saved in contacts has comma in it

Categories

(Core Graveyard :: DOM: Contacts, defect, P2)

ARM
Gonk (Firefox OS)

Tracking

(tracking-b2g:backlog, b2g-v2.2 affected, b2g-master affected)

RESOLVED WONTFIX
tracking-b2g backlog
Tracking Status
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: ericcc, Unassigned)

References

Details

Attachments

(1 file)

Attached image 2015-02-11-11-39-30.png
### STR
1. Have a number saved with extension like "0987654321,123" and name in Contacts.
2. Make call to this from contact details or manually key in dial pad(with ,).

### Actual 
Call screen not showing contact name.
Call log not showing contact name.
Removing ",123" from contact details resolve this.


### Expected
Call screen showing contact name.
Call log showing contact name.

### Version
Build ID               20150210162504
Gaia Revision          b6ca0aed54fb098f8c2ca2711a917ac351fc7380
Gaia Date              2015-02-10 18:20:34
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0f
4ebef7aeb1
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150210.194305
Firmware Date          Tue Feb 10 19:43:15 EST 2015
Bootloader             L1TC000118D0
Set 2.2?, breaking existing functionality
blocking-b2g: --- → 2.2?
Keywords: regression
Just tried 2.1. 

Findings: on 2.1, i can long press 6 to add comma and save contact but then, i can't dial that number.
it says, "the phone number you are calling is not valid".
I think it's a new feature bug 911055 enabling 2.2 to be able to make actual call.
Now the problem is that call log doesn't display the name accordingly.
We'll need more people's comment to triage for blocking/non-blocking decision.
Status: NEW → ASSIGNED
Target Milestone: --- → 2.2 S6 (20feb)
Assignee: nobody → gtorodelvalle
Like said in comment 2, the pause (aka "comma") has landed for 2.2 in bug 911055. This is likely not a regression then. Clearing the keyword.
Blocks: 911055
Keywords: regression
Triage: 
Noemi confirmed that this wont be a certification blocker and as such we should not block on this. But this is a bug that should be fixed and we should ask for pickup/approval when its fixed.

The fix is more involved than originally expected.
blocking-b2g: 2.2? → ---
Hi guys! Yeah, yeah, as Johan mentions in comment 3, this is not a regression. The use case mentioned in this bug was not considered or implemented in bug 911055. In https://bugzilla.mozilla.org/show_bug.cgi?id=911055#c5 , Beatriz mentions the need to dial these kind of numbers from the keypad or the Contacts app (which we currently support) but we did not consider showing the contact data in the Call Screen or the Call Log. In fact, related to this is Anthony's https://bugzilla.mozilla.org/show_bug.cgi?id=911055#c48

Back to this bug, the solution is not as straight forward as it may seem at first glance. The reason is the way the "playing of the pauses" is currently implemented on the Gecko and Gaia sides, which basically is as follows:
  1. In case of dialing a number with commas, in Gaia we navigator.mozTelephony.dial() the base number (until the first comma).
  2. Once the call is established (connected), in Gaia we navigator.mozTelephony.sendTones() each group of tones between the commas (there could be several groups) and the pauses between them.

The consequence of this is that when the RIL notifies an ongoing call (even before established), all it knows is the base number (no news about the commas or group of tones) and obviously it is this base number the one it "notifies" or "passes" to the Call Screen to be shown. No need to say that if there is a contact including the base number as an associated phone number, it is this contact the one shown. I have just confirmed this.

So, considering what we have without further changes involving the RIL and so on. There are different approaches to the problem:
 1. Instead of searching for exact matches of contacts having the called base number as an associated phone number, we could use the "startsWith" filtering operation. We should still consider all the variants madness. But anyhow, the main problem here is that we loose control on the results which may be returned as matches. I mean, any contact with a phone number starting with the base number (or any of its variants) would be a valid match. In fact, with this approach ANY called phone number would be searched for associated numbers using the "startsWith" filtering since the Call Screen does not have a way to know if the call is to a number with pauses or not. As a consequence, if there is a contact with the associated phone number "612345678" and the user dials "612,123,,4,5" the contact which would be shown is the one associated to "612345678" (or any other starting with "612"). I am sure you can envision more painful situations :S
 2. Another possible approach, being aware that it is not the cleanest one and may involve some magic to the user :p, could be to include as a valid phone number for any contact with a "number with pauses" phone number, the base number. I mean, if the user associates to a contact the phone number "612,123,,4,5", the Contact app would include as an additional associated phone number for this contact the base one ("612"). Yeah, I know, I know :p This would "magically" solve the showing of the contact everywhere (Call Screen and Call Log) since there is an exact matching number for the base number.
 3. Update navigator.mozTelephony.dial() to be able to deal with "number with commas". This would basically mean that from Gaia we would pass the whole number (with commas) to the RIL which would filter out any data from the first comma on and consequently would establish the call to the base number. The rest of the already existent code would keep on working regarding playing the tones and the pauses. The point would be (and I am not aware of the complexity of this) that the RIL would notify back to the Call Screen the whole number with commas which would let us manage it in Gaia accordingly (showing the correct associated contact, if any).

No need to say that the first 2 options are "regression bombs" :p and my vote would be for the third option.

Since the solution is not straight forward I am need-infoing Doug and Gabrielle to let them lead the solution to this issue.
See Also: → 1132444
[Blocking Requested - why for this release]: Renoming this bug for triage. This is a broken new functionality.
blocking-b2g: --- → 2.2?
triage: must fix for 2.2
blocking-b2g: 2.2? → 2.2+
Triage: further discussion with partner we need this feature to be part of the release. We do understand this is not an edge case and, even though it will not block partner test, we need to fix this for the new feature to work correctly.
Flags: needinfo?(gsvelto)
Flags: needinfo?(drs.bugzilla)
Considering the past history of Telephony.dial() I'd pick option 3 from comment 5. Let me describe the precedent and why I think this is what makes most sense: we used to have two separate functions to deal with regular numebers (dial) and MMI codes (sendMMI), gaia had some logic to decide when to use one and when to use the other. As time went by and use cases were added the gaia glue became more fragile and we encountered more and more problems which in turn required additional workarounds to use the correct function. Ultimately sendMMI() was deprecated and dial() now deals with MMI codes on its own; this is robust and ensures that gecko and gaia can't have different interpretations of both.

The problem here is somewhat similar in that we're using gaia to reinterpret a number and act upon it depending on the different situations. This is causing incoherence between gecko and gaia and there's really no robust way to solve it but to give gecko full control over what gets called so that all related events are coherent with it.
Flags: needinfo?(gsvelto)
I'm in full agreement with Gabriele in comment 9. However, Hsin-Yi is on vacation for CNY. I'm needinfoing her anyways.

I'd like to note that this could potentially become a very big change, and I strongly recommend against doing this after FL.

Wilfred, do you have any suggestions here? I'm inclined to just wait until Hsin-Yi is back, but under the circumstances, the sooner we get this done, the better.
Flags: needinfo?(wmathanaraj)
Flags: needinfo?(htsai)
Flags: needinfo?(drs.bugzilla)
Assignee: gtorodelvalle → nobody
Status: ASSIGNED → NEW
Target Milestone: 2.2 S6 (20feb) → ---
as discussed with Johann and partner today - the feature of being able to store a number in contacts (with pauses) and be able to dial that number is a critical feature.

The local countries of partners have been asking for this as this is often used for accessing specific service center options, bank dial ins, general useage in public. The user base that uses this is extensive - there are no exact number of people we can define plus there is no way to know what each person has on their device stored.

The bug that exists with the contact name not showing on the call log is essential to the feature but this should not block the feature release in 2.2. The number is still shown and available to the user and this is more than enough for this release.

The fix for displaying on call log can be done in an upcoming release (I just tried on Android 4.3 and there the same issue happens)  - not sure if this has been fixed in newest android release.
Flags: needinfo?(wmathanaraj)
Hello all, I am back from the new year holiday. Sorry for the late response.

If we are going to support this behaviour, I agree that we extend dial API. However, before we dive into the API design, could we review the UX spec first to make sure we don't miss anything, then we don't need to jump back and forth? 

Hi Wilfred, in addition to "dialling <<pause>>", do we consider to support another similar feature "dialling <<waiting>>"? IMO, we should consider the API for dialling waiting along with dialling pause. Dialling waiting requires user confirmation that leads to more complexity. If we are going to support this eventually, then I'd like to make sure the new API design for dialling pause is able to support dialling waiting, at least to be able to be a base of dialling waiting. Could you provide feedback here?

Per comment 11, this isn't a 2.2 blocker. I'd denominate it to get it properly triaged again, thank you all.
blocking-b2g: 2.2+ → 2.2?
Flags: needinfo?(htsai) → needinfo?(wmathanaraj)
Triage: See comment 11.
blocking-b2g: 2.2? → backlog
Duplicate of this bug: 1050648
blocking-b2g: backlog → ---
Flags: needinfo?(wmathanaraj)
Component: Gaia::Dialer → DOM: Contacts
Priority: -- → P1
Product: Firefox OS → Core
Not sure why this got moved to DOM: Contacts; do we have any evidence of an issue in the API?

I mean, it's definitely possible, but no earlier comment is talking about this.
(In reply to Julien Wajsberg [:julienw] (PTO -> Apr 27) from comment #15)
> Not sure why this got moved to DOM: Contacts; do we have any evidence of an
> issue in the API?

The issue here is that commas are used to express pauses and those are handled separately from the number in the dialer. So when you call a number with commas only the actual number will be passed to Telephony.dial(), in turn the telephony call ended message will carry this number alone preventing us from matching it with a contact that contained the whole number with commas and all. The solution here is to allow Telephony.dial() to cope with commas in numbers and then get rid of the special-casing we currently do in gaia.
Severity: normal → major
Priority: P1 → P2
DOM: Contacts isn't used anymore. 
Closing all remaining bugs.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.