829827 - STK Setup call confirmation does not display properly

Status: RESOLVED FIXED

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

Description

[Carol Yang [:cyang]]

In GCF TC 27.22.4.13.1.5, the test is ran as follows:

- Start GCF test which prompts for device power up
- After power up and network registration is successful, GCF test prompts to call 321
- After call to 321 connects, GCF test will send a SET_UP_CALL proactive command to display "Disconnect" confirmation message

At this point, nothing is displayed. I noticed that after I hang up the call to 321, the dialer goes away and then now the "Disconnect confirmation message is seen.

This is sent when GCF test issues SET_UP_CALL proactive command:

RILContentHelper: Received message 'RIL:StkCommand': {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":{"confirmMessage":"Disconnect","address":"012340123456,1,2"}}

Comment 1

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

This is because the diconnect message is recived by the settings app which is in background (since the dialer is of the fg)

Which should be the theoretical way of working?

Comment 2

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #1)
> This is because the diconnect message is recived by the settings app which
> is in background (since the dialer is of the fg)
> 
> Which should be the theoretical way of working?

According to 3GPP TS 11.14, section 12.6, the command qualifier can indicate the following:

SET UP CALL:
'00' = set up call, but only if not currently busy on another call;
'01' = set up call, but only if not currently busy on another call, with redial;
'02' = set up call, putting all other calls (if any) on hold;
'03' = set up call, putting all other calls (if any) on hold, with redial;
'04' = set up call, disconnecting all other calls (if any);
'05' = set up call, disconnecting all other calls (if any), with redial;
'06' to 'FF' = reserved values.

Could this information be used to help determine which take precedence?

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

CC Edger,
please help to check if gecko needs provide other information to gaia.

Comment 4

[Edgar Chen [:edgar]]

Although we have exposed commandQualifier in MozStkCommand, but I think it is better to have a new property in MozStkSetUpCall determine which action shall be performed as mention in comment #2.

Hi Fernando and Yoshi, how do you think?

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #4)
> Although we have exposed commandQualifier in MozStkCommand, but I think it
> is better to have a new property in MozStkSetUpCall determine which action
> shall be performed as mention in comment #2.
> 
> Hi Fernando and Yoshi, how do you think?

Go ahead!

Comment 6

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Yoshi, I tested this on my handset and after user confirmation the response is sent to the icc object which should sent the call.

I'm not sure, but the address is a number with commas, take care with it ;)

E/GeckoConsole(  389): Content JS LOG at app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings] sendStkResponse to command: {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":{"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
E/GeckoConsole(  389): Content JS LOG at app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings] sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}

-> I reassign the bug to platform.

Comment 7

[Andrew Overholt [:overholt]]

So is this supposed to be in Gaia or Gecko now?

Comment 8

[Daniel Coloma:dcoloma]

Based on comment 6 I would say it is on Gecko.

Comment 9

[Andrew Overholt [:overholt]]

Okay.  I imagine Yoshi will be online in the next few hours and comment or work on a patch :)

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Fernando R. Sela [:frsela] from comment #6)
> Yoshi, I tested this on my handset and after user confirmation the response
> is sent to the icc object which should sent the call.
> 
> I'm not sure, but the address is a number with commas, take care with it ;)
> 
> E/GeckoConsole(  389): Content JS LOG at
> app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> sendStkResponse to command:
> {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":
> {"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
> E/GeckoConsole(  389): Content JS LOG at
> app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}
> 
> -> I reassign the bug to platform.

Hi, frsela

Can you explain with more detail why it's a platform bug ?

The bug is when user dialling to 321, then icc sends a proactive command,
but Gaia doesn't show that message to user.

The address is correct, ',' is the pause character in GSM, it's defined in TS 151.011, clause 10.5.1, extended BCD number.
 
Currently you can use commandQualifier in command to decide what to do, but for better API use, Edgar has filed a bug 830696 to achieve this.

Reassign this bug back to you, as I think this is gaia bug,
or if you still think this is platform bug, can you help to provide more information about you opinion?

Thanks

Comment 11

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #10)
> (In reply to Fernando R. Sela [:frsela] from comment #6)
> > Yoshi, I tested this on my handset and after user confirmation the response
> > is sent to the icc object which should sent the call.
> > 
> > I'm not sure, but the address is a number with commas, take care with it ;)
> > 
> > E/GeckoConsole(  389): Content JS LOG at
> > app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> > sendStkResponse to command:
> > {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":
> > {"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
> > E/GeckoConsole(  389): Content JS LOG at
> > app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> > sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}
> > 
> > -> I reassign the bug to platform.
> 
> Hi, frsela
> 
> Can you explain with more detail why it's a platform bug ?
> 
> The bug is when user dialling to 321, then icc sends a proactive command,
> but Gaia doesn't show that message to user.
> 
> The address is correct, ',' is the pause character in GSM, it's defined in
> TS 151.011, clause 10.5.1, extended BCD number.
>  
> Currently you can use commandQualifier in command to decide what to do, but
> for better API use, Edgar has filed a bug 830696 to achieve this.
> 
> Reassign this bug back to you, as I think this is gaia bug,
> or if you still think this is platform bug, can you help to provide more
> information about you opinion?
> 
> Thanks

Because I understood that the call is not dialed.

On the UI I:

show the confirmMessage and after user confirmation I sent the ICC response with the hasConfirmed attribute.

I tested the UI and is showing and working as expected. Could you check if the call is dialed after this response?

      case icc.STK_CMD_SET_UP_CALL:
        debug(' STK:Setup Phone Call. Number: ' + options.address);
        var msg = '';
        if (options.confirmMessage) {
          msg += options.confirmMessage;
        }
        var confirmed = confirm(msg + ' ' + options.address);
        iccLastCommandProcessed = true;
        responseSTKCommand({
          hasConfirmed: confirmed,
          resultCode: icc.STK_RESULT_OK
        });

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi Frsela,

Sorry I got confused about what you said.

Carol, if I am not correct please also help to provide your comments.

From description.

1. User dials "321" first.
2. Then icc sends proactive command, with SET_UP_CALL.

But now I think the bug is the dialog of SET_UP_CALL confirmation didn't pop up now.

Quoted from Carol
"- After call to 321 connects, GCF test will send a SET_UP_CALL proactive command to display "Disconnect" confirmation message

At this point, nothing is displayed."

I think this bug is not you sent the TERMINAL RESPONSE or not, it's about "Display the confirm message when user is dialling.

Comment 13

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Carol Yang from comment #2)
> According to 3GPP TS 11.14, section 12.6, the command qualifier can indicate
> the following:
> 
> SET UP CALL:
> '00' = set up call, but only if not currently busy on another call;
> '01' = set up call, but only if not currently busy on another call, with
> redial;
> '02' = set up call, putting all other calls (if any) on hold;
> '03' = set up call, putting all other calls (if any) on hold, with redial;
> '04' = set up call, disconnecting all other calls (if any);
> '05' = set up call, disconnecting all other calls (if any), with redial;
> '06' to 'FF' = reserved values.
> 
> Could this information be used to help determine which take precedence?

Hi, Carol
As we found Android doesn't handle the Command Qualifier from SET_UP_CALL,
does Firefox OS also need to support this?

Thanks

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

Triage: mvines, could you confirm if this is a blocker or not base on comment 13?

Comment 15

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #12)
> Hi Frsela,
> 
> Sorry I got confused about what you said.
> 
> Carol, if I am not correct please also help to provide your comments.
> 
> From description.
> 
> 1. User dials "321" first.
> 2. Then icc sends proactive command, with SET_UP_CALL.
> 
> But now I think the bug is the dialog of SET_UP_CALL confirmation didn't pop
> up now.
> 
> Quoted from Carol
> "- After call to 321 connects, GCF test will send a SET_UP_CALL proactive
> command to display "Disconnect" confirmation message
> 
> At this point, nothing is displayed."
> 
> I think this bug is not you sent the TERMINAL RESPONSE or not, it's about
> "Display the confirm message when user is dialling.

So this could be cause the dialer is on the frontend. So the question is:

If SET_UP_CALL is received SHALL be managed if FOREGROUND when another call is on going?

Comment 16

[Michael Vines [:m1] [:evilmachines]]

(moving ni? to cyang right now as she's in the process of digging through android stuff to validate the assertion made in comment 13)

Comment 17

[Carol Yang [:cyang]]

As Yoshi mentioned in Comment 13, looks like Android does not handle Command Qualifier.

However, I believe modem will take care of that. By this, I mean the STK proactive command for SET_UP_CALL will not come if Command Qualifier specifies so, i.e. 0x00, and it will come when Command Qualifier specifies so, i.e. 0x04. I need to verify this particular comment still and will update when I find out.

For now, still seems like this current bug needs to be addressed since modem is doing the filtering.

Comment 18

[Daniel Coloma:dcoloma]

(In reply to Carol Yang from comment #17)
> As Yoshi mentioned in Comment 13, looks like Android does not handle Command
> Qualifier.
> 
> However, I believe modem will take care of that. By this, I mean the STK
> proactive command for SET_UP_CALL will not come if Command Qualifier
> specifies so, i.e. 0x00, and it will come when Command Qualifier specifies
> so, i.e. 0x04. I need to verify this particular comment still and will
> update when I find out.
> 
> For now, still seems like this current bug needs to be addressed since modem
> is doing the filtering.

so... who should be the owner of this bug?

Comment 19

[Anshul]

So the issue is not handling of command qualifiers as in bug "https://bugzilla.mozilla.org/show_bug.cgi?id=830696" but the fact that the user confirmation dialog is shown underneath the OnCall UI and so not visible to the user to take any action.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Anshul from comment #19)
> So the issue is not handling of command qualifiers as in bug
> "https://bugzilla.mozilla.org/show_bug.cgi?id=830696" but the fact that the
> user confirmation dialog is shown underneath the OnCall UI and so not
> visible to the user to take any action.

Without verify the behavior with UX, you should be able to use reopenApp() trick here to bring the Settings app to the foreground.

https://github.com/alivedise/gaia/blob/9058317544abe7b4759c6535e1471c65ed9ef71f/apps/communications/dialer/js/keypad.js#L281-L286

Let me know if it works.

Comment 21

[Carol Yang [:cyang]]

Just FYI. I've verified that the modem supports all the command qualifiers mentioned in Comment 2. So if a proactive command for SET_UP_CALL comes, that means an option for the user to confirm/reject the call should always be displayed to the user.

Comment 22

[Anshul]

Any update on this issue?

Comment 23

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Move settings to foreground on any STK command

A patch to move settings to foreground on any STK commads as suggested by :timdream on comment 20.

Carol, can you check this works and pass GCF?

Comment 24

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 706277 [details]
Move settings to foreground on any STK command

But remove the additional 'settings' stuff.

Comment 25

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Landed: https://github.com/mozilla-b2g/gaia/commit/bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0

Comment 26

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

Uplifted to v1-train: 
https://github.com/mozilla-b2g/gaia/commit/b43e6e08289cbecea4d5b443155c8f16645e79f1

Uplifted to v1.0.0:
https://github.com/mozilla-b2g/gaia/commit/71061d8a7fd6b664e0e0b7559263aed3bee7a1ab

Comment 27

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #23)
> Created attachment 706277 [details]
> Move settings to foreground on any STK command
> 
> A patch to move settings to foreground on any STK commads as suggested by
> :timdream on comment 20.
> 
> Carol, can you check this works and pass GCF?

Hi Fernando,

Looks like this patch will bring up the SET_UP_CALL confirmation dialog. The GCF test hasn't passed yet but we're still looking into whether the remaining issues is gaia or not. Either way, the purpose of this bug has been resolved.

Comment 28

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Carol Yang from comment #27)
> (In reply to Fernando R. Sela [:frsela] from comment #23)
> > Created attachment 706277 [details]
> > Move settings to foreground on any STK command
> > 
> > A patch to move settings to foreground on any STK commads as suggested by
> > :timdream on comment 20.
> > 
> > Carol, can you check this works and pass GCF?
> 
> Hi Fernando,
> 
> Looks like this patch will bring up the SET_UP_CALL confirmation dialog. The
> GCF test hasn't passed yet but we're still looking into whether the
> remaining issues is gaia or not. Either way, the purpose of this bug has
> been resolved.

\o/ Thanks

Comment 29

[Carol Yang [:cyang]]

Hi Fernando,

This change looks to only be partially merged in. The changes in utils.js are there but I don't see the call to reopenSettings() being made in https://github.com/mozilla-b2g/gaia/blob/v1.0.0/apps/settings/js/icc.js. Do you know if this will just take some time or if something has gone wrong with the merge?

Comment 30

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Carol Yang from comment #29)
> Hi Fernando,
> 
> This change looks to only be partially merged in. The changes in utils.js
> are there but I don't see the call to reopenSettings() being made in
> https://github.com/mozilla-b2g/gaia/blob/v1.0.0/apps/settings/js/icc.js. Do
> you know if this will just take some time or if something has gone wrong
> with the merge?

We should check what happened in the v1.0.0 merge, but in master branch is ok (https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/icc.js#L208) and as you point out, in v1.0.0 is not completely merged or was removed in a future commit.

Some bisect in the v1.0.0 branch told me that the "evil" commit is this one: https://github.com/mozilla-b2g/gaia/commit/c47396cc278918acc1798b513cab9dc21d0f83f5

Comment 31

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Fernando R. Sela [:frsela] from comment #30)

> Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> https://github.com/mozilla-b2g/gaia/commit/
> c47396cc278918acc1798b513cab9dc21d0f83f5

One thing more, in master branch, the 7799 PR (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged AFTER so probably the merge tool differed both and removed the previous commited code.

Comment 32

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #31)
> (In reply to Fernando R. Sela [:frsela] from comment #30)
> 
> > Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> > https://github.com/mozilla-b2g/gaia/commit/
> > c47396cc278918acc1798b513cab9dc21d0f83f5
> 
> One thing more, in master branch, the 7799 PR
> (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one
> - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged
> AFTER so probably the merge tool differed both and removed the previous
> commited code.

Thanks for looking into this Fernando! What's next? What's the process to get 7798 properly/fully merged into v1.0.0 (and looks like v1-train as well) given the merge error already occurred?

Comment 33

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

I uplifted the merged-over part of the patch with v1-train@ccb490e and v1.0.0@3f505ae

Comment 34

[Carol Yang [:cyang]]

(In reply to John Ford [:jhford] from comment #33)
> I uplifted the merged-over part of the patch with v1-train@ccb490e and
> v1.0.0@3f505ae

Thanks John! I see it now.

Comment 35

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Carol Yang from comment #32)
> (In reply to Fernando R. Sela [:frsela] from comment #31)
> > (In reply to Fernando R. Sela [:frsela] from comment #30)
> > 
> > > Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> > > https://github.com/mozilla-b2g/gaia/commit/
> > > c47396cc278918acc1798b513cab9dc21d0f83f5
> > 
> > One thing more, in master branch, the 7799 PR
> > (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one
> > - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged
> > AFTER so probably the merge tool differed both and removed the previous
> > commited code.
> 
> Thanks for looking into this Fernando! What's next? What's the process to
> get 7798 properly/fully merged into v1.0.0 (and looks like v1-train as well)
> given the merge error already occurred?

We wrote to :jhford explaining the issue. He fixed it (as he answered before me ;)

Thanks John.

Comment 36

[amiller]

No need to create a TC in Moztrap for this defect.