990369 - [Dialer] update call number during call state transition

Status: RESOLVED FIXED

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

Description

[Hsin-Yi Tsai (she/her) [:hsinyi]]

In STK Call Control scenario, a call number may change during call state transition, normally changing before call state becomes CONNECTED.

File this bug for Dialer to take care of this.

Comment 1

[Anthony Ricaud (:rik)]

The fix should be in apps/communications/dialer/js/handled_call.js in HandledCall.prototype.connected. We should update the DOM and the recentsEntry.

Good unit tests will help check this is working.

Comment 2

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

i am taking this mentor bug.

Comment 3

[Doug Sherk (:drs) (inactive)]

Unassigning as there has been no progress for several months.

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Josh, should we nominate this as 2.0M+ as this is the gaia part for bug 986308, which is 2.0m+? Without this gaia patch, the stk call number feature isn't completed.

Comment 5

[Josh Cheng [:josh]]

Hi Hsin-Yi,
Thank you very much for the heads up. Do you know who can take this bug?

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Josh Cheng [:josh] from comment #5)
> Hi Hsin-Yi,
> Thank you very much for the heads up. Do you know who can take this bug?

Maybe we can ask dialer owner Doug (:drs) first :)

Comment 7

[Josh Cheng [:josh]]

Dear Doug,
Could you please help to take this bug as this is UI part fix of bug 1089447?
Thank you!

Comment 8

[Tamara Hills [:thills]]

Hi Hsin-Yi,

I'm looking at this and I looked the gecko patch for https://bugzilla.mozilla.org/show_bug.cgi?id=986308 and I'm wondering if you have a link or pointer to some instructions on how I can reproduce the actual problem with the network emulator?  Also, is there any way to test this out on a device so I can test my actual changes work?

I'm a little unclear about the actual problem.  My understanding so far is that an outbound call is made to a particular number and the number is changed while in the alerting state.  I'm supposing this can be from an IVR automated system.  Is this correct?

Thanks,

-tamara

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Tamara Hills [:thills] from comment #8)
> Hi Hsin-Yi,
> 
> I'm looking at this and I looked the gecko patch for
> https://bugzilla.mozilla.org/show_bug.cgi?id=986308 and I'm wondering if you
> have a link or pointer to some instructions on how I can reproduce the
> actual problem with the network emulator?  

Sorry, the emulator support isn't done yet. Please see Bug 1091354.

> Also, is there any way to test
> this out on a device so I can test my actual changes work?
> 

It depends on sim cards, but I am not really sure which sim card does have this feature. :(

The way I tested was to hack gecko code to trigger call number change in [1]. For example, having one line |"newCall.number = "112";| right before [1].

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#3986

> I'm a little unclear about the actual problem.  My understanding so far is
> that an outbound call is made to a particular number and the number is
> changed while in the alerting state.  I'm supposing this can be from an IVR
> automated system.  Is this correct?
> 

Sort of ;)

The problem is some operators have "STK call control" mechanism in their sim cards. If a sim card supports this feature, when user dials a call, the device will check rules stored in a sim card. Possible results are |nothing needs to change, dial out directly|, |dial allowed with number changed|, |dial forbidden|, etc. So, this really depends on every carrier and their sim features.

Hope it's clearer now. :)

> Thanks,
> 
> -tamara

Comment 10

[Josh Cheng [:josh]]

Hi Tamara, Hi Xinyi,
Thanks for your help. Since we do not have special SIM card. I can ask partner to test patch first before we land patch. Thanks!

Comment 11

[Josh Cheng [:josh]]

Hi Tamara, 
This bug is requesting urgently by partner because STK is certification issue. Do you have any update? Thanks!

Comment 12

[Evelyn Hung]

Hi Sean, please see if you can help out here. Thanks!

Comment 13

[Tamara Hills [:thills]]

Hi Josh,

I have this as my top priority and am working through Hsin-Yi's suggestion right now.  I will post as soon as I have a patch that can be tested.

Thanks,

-tamara

Comment 14

[Josh Cheng [:josh]]

Dear Tamara, 
Thank you very much for your help and really appreciate!

Comment 15

[Tamara Hills [:thills]]

Attachment: WIP for quick test.

Hi Josh,

Can we have them try this out to see if this works?  I tried Hsin-Yi's suggestion and I was able to see the number change when connected.  I want to make sure I'm on the right track before fixing some of the surrounding items and tests.

Thanks,

-tamara

Comment 16

[Josh Cheng [:josh]]

Hi PengFei,
Could you help to verify the patch? This is the gaia part of bug 1089447. Thanks!

Comment 17

[pengfei.huang]

Hi Josh,
   The patch work well.It update call number on call screen after call control command.The log you can see bug 1089447 comment26.
   Thanks.

Comment 18

[Josh Cheng [:josh]]

Hi Tamara,
Thanks for your help! Can you initiate patch review process?

@PengFei,
Thank you!

Comment 19

[pengfei.huang]

Attachment: 12-18-mtklog-callcontrol.zip

Hi Josh,
   I am unwilling to tell you there is a little problem. Our VAL co-workers don't agree "112 emergency" call screen entirely. They demand should has the words "Emergency number"  above the 112 number.Like the screenshots I provided. And I add the log attachment too.Could please help us solve it perfectly?
   Thank you very much.

Comment 20

[Tamara Hills [:thills]]

Hi Josh/PengFei,

I'm working on the changes for the 'emergency number' to show up as per your screenshot.  I still have it as my top item, so I will leave the NI for myself as I'm just trying to make sure I test this properly without dialing 911.

Carrie, do you mind attaching a spec just so I can make sure my changes are correct as I'm seeing a few discrepancies between PengFei's screenshot and what's in the code and I want to make sure.  I just need a screen shot of what the call screen shoudl look like when dialing an emergency number.

Thanks,
-tamara

Comment 21

[Tamara Hills [:thills]]

Attachment: WIP v2

Hi Josh,

Can you have them try out this version?  I changed the screen to look like the screenshot and have "Emergency number" at the top with the number (would be 112 in this case) below.

I still need to hear back from Carrie on this.  In the meantime I will move forward with the tests based on this.

Thanks,

-tamara

Comment 22

[Josh Cheng [:josh]]

Dear PengFei,
Can you try patch per comment 21? Thanks!

Comment 23

[pengfei.huang]

Attachment: 12-18-mtklog-call-control_v2.zip

Hi Tamara,
   I try you patch and occurred an exception.
Exception: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIFile.append]"  nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> xxx/gaia/build/webapp-shared.js :: WebappShared.prototype.pushLocale :: line 241"  data: no]

  And I guess the problem caused by below code.  We have different way to implement localization. So, I remove code below and test(if any affect, please let me know.):
    <link rel="localization" href="shared/locales/telephony/telephony.{locale}.properties">
  The dialing screen has problem, the word "Connecting" is gone.you can check screenshots and log in the attachment.

Comment 24

[Tamara Hills [:thills]]

Hi Pengfei,

Thanks for testing out.  I added that telephony.{locale}.properties to get the emergencyNumber string localized (so we get the 'Emergency number' string that was requested).  That is defined in telephony.{locale}.properties.  

I did not see the errors that you saw.  Can you explain little bit about how we work the localizations for you? Is there a different method that you can use to localize the emergencyNumber string?  Do you usually make these types of changes to make localizations work?  

One more question is when you remove the telephony.properties file from index.html, does the "Connecting" string come back?

Thanks,
-tamara

Comment 25

[pengfei.huang]

Hi Tamara,
   There is no telephony{local}.properties in our project. We replace it by /callscreen/locales/locales.ini file. In that file, we import several localizations properties files because we need to translate strings to several languages. And in the code, we call "_" function to get translated string. I guess it's ok for your patch.
   And My test is without telephony.properties file from index.html yesterday. I am finding out the root cause why the word is gone, please help us too.Thank you.

Comment 26

[Tamara Hills [:thills]]

Hi Carrie or Wilfred,

Do you have a spec of what the call screen is supposed to look like during an emergency call?  Can you please provide as the current behavior is different from what is being requested by this bug and I'm hesitant to make such changes in the emergency area without having a spec.

Thanks, 

-tamara

Comment 27

[pengfei.huang]

Hi Tamara & Josh,
   I change a device with your patch comment21, and I remove the code "this.updateCallNumber()" on HandledCall.prototype.remove which you add then I add "this.updateCallNumber()" on the end of method HandledCall.prototype.connected. It works well, we can see the "emegency number" words. Please help us try it is right for you. Thanks.

Comment 28

[Carrie Wang [New account: [e-mail]]]

Hi Tamara, 

Sorry for the delay.
From bug 1086206, 
https://bugzilla.mozilla.org/show_bug.cgi?id=1086206
We already provide the string "Emergency numbers" on the call screen. 
I haven't received the related requests since I took over the Dialer UX. So I'd suppose this was defined long time ago. 
I can't see the string appearing on my device either. So can you please check the Dialer code to see if this was implemented? I think the attachment "index.png" could be a reference if the feature doesn't exist and you can ni Carol to check the string size and font. Thanks!

Comment 29

[Tamara Hills [:thills]]

Hi Pengfei,

Which branch are you applying the patch on top of?  It the updateCallNumber should be at the end of the connected method, not remove.  My patch was on top of the mozilla-central branch.

Also, one thing I wanted to point out is whether you have a requirement for which line should say 'Emergency number'.  There is the top line and then the bottom line.  If I look at the screen shot Carrie refers to in comment 28, the 'Emergency number' is on the bottom.  In your screenshot it's on the top.  Is it acceptable for us to keep it how it is in Carrie's screenshot?

Thanks,

-tamara

Comment 30

[pengfei.huang]

Hi Tamara,
   I merged your patch WIP v1 comment15 and WIP v2 comment 21. The updateCallNumber I removed is on method HandledCall.prototype.remove. And there is no code "updateCallNumber" on method HandledCall.prototype.connected before or after merging your patch.
   There is no requirement for the 'Emergency number'. We remain it's appearance the same as when we dial emergency call directly.

Comment 31

[Tamara Hills [:thills]]

Hi Pengfei and Josh,

Can you please explain how you are building and what branch you are building?  I can't see the problem that you are seeing right now on the master branch with my patch.  Are you building a 2.0 branch, 2.0m?  

Thanks,

-tamara

Comment 32

[pengfei.huang]

Hi Tamara,
   we build on 2.0m. I don't know what exactly different 2.0 branch and 2.0m. We build with make command. The more detail I need to ask integration co-worker.

Comment 33

[Josh Cheng [:josh]]

Hi Tamara,
We are using 2.0M branch. However the problem is generic and also found on 2.2 master. Is your patch also good for 2.0M?
Thanks!

Comment 34

[pengfei.huang]

Attachment: 12-26-mtklog_call.zip

Hi Josh,
   Do you remember the call control command with Alpha Identifier "Allowed with Modifications" which we haven't fixed yet. We need to show the Alpha Identifier to user.
   I dig the log and can't find the information about Alpha Identifier. Could you help finding the root cause?

Comment 35

[Josh Cheng [:josh]]

Hi PengFei,
Do you have Bug ID for the issue you mentioned? Thanks!

Comment 36

[pengfei.huang]

Hi Josh,
   I think the id is bug1089447. I remember it filed by MTK with eService ALPS01771672 and Mozilla combined 1089447 with it. Should I file another one?
   Thanks.

Comment 37

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to pengfei.huang from comment #34)
> Created attachment 8541541 [details]
> 12-26-mtklog_call.zip
> 
> Hi Josh,
>    Do you remember the call control command with Alpha Identifier "Allowed
> with Modifications" which we haven't fixed yet. We need to show the Alpha
> Identifier to user.
>    I dig the log and can't find the information about Alpha Identifier.
> Could you help finding the root cause?

Hi Pengfei,
I am not sure if I got your question. Could you elaborate on what exactly string or symbols you'd like to see in the main log? Were you saying some string such as "allowed with modifications"? If so, it's normal that we don't see it in gecko main log because the underlying STK call control mechanism is handled on modem, not gecko. What gecko does is to convey the number gecko receives from modem to gaia.

In addition, from this attachment, I saw the numbers 0155663732 and [phone] but no emergency number 112. Which number were we supposed to see?  Thank you.

Comment 38

[pengfei.huang]

Attachment: 12-30-call_control_patch.zip

Hi Josh,
  It's the patch I test PASS, please check. Hope it's good for you.
  Thanks

Comment 39

[pengfei.huang]

Hi Hsin-Yi Tsai,
  The attachment is the log of testing call control with Alpha Identifier, which numbers 0155663732 change to [phone] with Alpha Identifier "allowed with modifications". And talking with Josh just now, he confirms it needs MTK handling.
  Thanks.

Comment 40

[Josh Cheng [:josh]]

Hi Gary,Can you help to check the patch per comment 38? Thank you!

Comment 41

[Josh Cheng [:josh]]

Hi Pengfei,
Per our discussion. Can you try repo the issue with Mozilla Gaia/Gecko? Thanks!

User build image link is: http://pan.baidu.com/s/1hqnDBtM
Access code: 9c9u
unzip password: erT14moz

Comment 42

[Tamara Hills [:thills]]

Attachment: 2m gaia patch

Hi Josh/Pengfei,

Sorry for the delay.  I was off for few days due to holidays.  I have a version that I hope will apply cleanly to 2.0m.  Can you give this a try?

Thank you,

-tamara

Comment 43

[Josh Cheng [:josh]]

Hi Pengfei,
Can you provide the result which you tried our code?

Tamara has provided new patch as comment 42. If our code not working. Can you try it if previous test without patch not working?
Thanks!

Comment 44

[pengfei.huang]

Hi Josh,
   Sorry for the delay due to corporation activity. Can you provide the method of flashing? I think we use different way with Mozilla. Many thinks.

Comment 45

[Josh Cheng [:josh]]

Hi Pengfei,
Please check the document here about how to flash our build.
Link: http://pan.baidu.com/s/1jGikrUQ 

Thanks!

Comment 46

[pengfei.huang]

Attachment: 12_31_screenshots_call_control.zip

Hi Josh,
   I try your image. And the attachment is the screenshots of testing call control.
   As the screenshots, we can see the call number never change after executing call control command . So, I guess your image is without the patch of comment15. We need the patch which fix number can change to 112 so that we can see if exist the word 'emergency number' on your image.
   Many thanks.

Comment 47

[Josh Cheng [:josh]]

(In reply to pengfei.huang from comment #46)
> Created attachment 8542800 [details]
> 12_31_screenshots_call_control.zip
> 
> Hi Josh,
>    I try your image. And the attachment is the screenshots of testing call
> control.
>    As the screenshots, we can see the call number never change after
> executing call control command . So, I guess your image is without the patch
> of comment15. We need the patch which fix number can change to 112 so that
> we can see if exist the word 'emergency number' on your image.
>    Many thanks.

Hi PengFei,
The patch is at comment 42. Can you help to try again? Thanks!

Comment 48

[pengfei.huang]

Hi Tamara,
   Before trying your patch comment42, should I ignore all your old path(comment15 and comment21)?

Comment 49

[Tamara Hills [:thills]]

Attachment: Pull Request for 2.2

Comment 50

[Tamara Hills [:thills]]

(In reply to pengfei.huang from comment #48)
> Hi Tamara,
>    Before trying your patch comment42, should I ignore all your old
> path(comment15 and comment21)?

Hi Pengfei,

Yes, that's correct.  The patch in comment42 is built from 2.0m and should apply to this one.

Comment 51

[pengfei.huang]

Hi Tamara,
  Your patch works well. We can see the 'emergency number' and call number changed. Pass the test.
  Many thanks.

Comment 52

[Josh Cheng [:josh]]

Hi PengFei,
Thanks for your great news.

Dear Tamara,
Great thanks from Woodduck team for your help during holidays!
Thank you!

Comment 53

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8542846 [details] [review]
Pull Request for 2.2

Clean and very well done. Thanks for driving this!

Comment 54

[Josh Cheng [:josh]]

Hi Kai-Zhen,
COuld you please help to land this patch on 2.0M? Thanks!

@Tamara, Could you also provide patch for m/c? Thanks!

Comment 55

[Kai-Zhen Li [:seinlin][:kli]]

master: https://github.com/mozilla-b2g/gaia/commit/bc0036116ab2f66dd56bf85e64c879b462311a46
v2.0m: https://github.com/mozilla-b2g/gaia/commit/06b544f8a9ee03754dd3f5020f279cffa7a75804

Please request the approval for v2.1 if it is necessary. Thanks!

Comment 56

[Tamara Hills [:thills]]

Hi Josh,

Clearing the NI as it looks like Kai-Zhen has landed the m-c patch for me.  Thanks.
-ta,ara