826508 - STK app no option to terminate SIM session

Status: RESOLVED FIXED

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

Description

[Carol Yang [:cyang]]

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

- Start GCF test which prompts for device power up
- After power up, hitting Continue from GCF test will send a proactive command to get inkey

At this point, the abort message is displayed but there are no options to terminate the SIM session, causing this GCF test to fail.

Comment 1

[Anshul]

Carol, should there be an UI button to terminate the SIM session after the abort message or is that something Gaia should trigger without any user interaction?

Comment 2

[Carol Yang [:cyang]]

Anshul- There should be a UI button (soft key) that allows the user to terminate the SIM session at any time.

Comment 3

[Carol Yang [:cyang]]

On an Android device, the way to end a STK session is to press the Options button and then a list of selection would show up there. The "End Session" option is a soft button on the UI which will always be available from this list. Upon pressing this, it would allow a user to end the Proactive SIM session.

Comment 4

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

Siddartha told me he could work on this.

Comment 5

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

Hi Fernando
Can you forward this information to UX?

User can close this STK session anytime, but how?

Comment 6

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

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #5)
> Hi Fernando
> Can you forward this information to UX?
> 
> User can close this STK session anytime, but how?

CCing Rafael Rebolleda.

Now you only can close STK session on main menu screen, the rest will only go back.

I think close at any time brokes the settings UX, but this is a Rafael decission.

Comment 7

[psiddh]

https://github.com/mozilla-b2g/gaia/pull/7553

Comment 8

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

(In reply to Siddartha P from comment #7)
> https://github.com/mozilla-b2g/gaia/pull/7553

I'm not sure if this is what Carol asked for. If I understood well a CLOSE button is needed in all screens but this brokes UX.

I pinged Rafael again to check this bug asap.

Comment 9

[psiddh]

All that the test case says is that there should be an option to send "uicc session terminated by user" by a "certain" user action. Now this "certain" user action can be left to different interpretations, but in FF OS case, pressing 'Home' button during a GetInkey / GetInput ongoing session is certainly qualified as a valid user action to inform that "uicc session terminated by user". Hope this clarifies. Correct me if I am wrong !

Comment 10

[Alexandre Poirot [:ochameau]]

Siddartha, I think that your branch is based on nightly gaia branch instead of master.
Could you rebase your patch against master?
$ git fetch upstream
$ git checkout your-branch
$ git rebase -i upstream/master
$ git push your-remote your-branch -f

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 7553

Looks good to me. It allows to info stk app that we are going off from the settings app and act accordingly.

Comment 12

[Alexandre Poirot [:ochameau]]

Comment on attachment 701117 [details]
Pull request 7553

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Would allow passing the test and be able to come up to home stk menu when leaving the settings app and coming back.
Testing completed: on device with orange stk-enabled simcard
Risk to taking this patch (and alternatives if risky):

Comment 13

[Carol Yang [:cyang]]

I pulled in attachment 701117 [details] to test. For the TC that this bug was filed on (27.22.4.2.1.4), pressing the home button will issue a terminate session terminal response. This is great and will allow the TC to pass.

However, is there anyway this solution can be more generic for other proactive commands? There are other GCF TC which requires terminate session which can benefit from sendSessionEndTROnFocusLose().

Comment 14

[psiddh]

Well I have thought about it. Since some commands such as SETUP MENU, SEND SMS, SEND SS / USSD etc SHOULD NOT send this terminal response at all to the UICC card, adding a common function means we may have to have 'if checks' such as
if (!  SETUP NENU || ! SEND SS ! || SEND SMS)  sendSessionTR(..)

On the contrary, if we can simple attach 'visibility' event listener just in the UI commands that we are interested in, we may have couple of redundant statements (single line statements though) in couple of other functions such Display Text, Select Item , PlayTone etc..

Since its a matter of preference , I chose latter than former. But if you think former makes sense, I am OK with it too. 
* I see a similar pattern for 'BACKWARD SESSION BY USER' setup at the common place, but not proactive commands should be sending this terminal response. We may have to check for commands that ought be sent this response upon back key selection from user.

Comment 15

[Alexandre Poirot [:ochameau]]

(In reply to Carol Yang from comment #13)
> However, is there anyway this solution can be more generic for other
> proactive commands? There are other GCF TC which requires terminate session
> which can benefit from sendSessionEndTROnFocusLose().

Is there any bug open for those other GCF TC tests failures? Or did you meant fixing all of those tests through this bug?

Siddartha, you may eventually register a global visibility change listener on startup and check `iccLastCommand` global and eventually dispatch a session-end by checking iccLastCommand against a command white-list.
In any case, please rebase your branch against master.

Comment 16

[Carol Yang [:cyang]]

(In reply to Siddartha P from comment #14)
> Well I have thought about it. Since some commands such as SETUP MENU, SEND
> SMS, SEND SS / USSD etc SHOULD NOT send this terminal response at all to the
> UICC card, adding a common function means we may have to have 'if checks'
> such as
> if (!  SETUP NENU || ! SEND SS ! || SEND SMS)  sendSessionTR(..)
> 
> On the contrary, if we can simple attach 'visibility' event listener just in
> the UI commands that we are interested in, we may have couple of redundant
> statements (single line statements though) in couple of other functions such
> Display Text, Select Item , PlayTone etc..
> 
> Since its a matter of preference , I chose latter than former. But if you
> think former makes sense, I am OK with it too. 
> * I see a similar pattern for 'BACKWARD SESSION BY USER' setup at the common
> place, but not proactive commands should be sending this terminal response.
> We may have to check for commands that ought be sent this response upon back
> key selection from user.

Found a pretty useful table in section 6.11 of the 3GPP TS 11.14. It shows you the possible terminal responses that can be returned for each proactive command. Looks like there are only 7 cases for which terminate session can be sent:

- SET_UP_CALL
- SEND_DTMF
- PLAY_TONE
- DISPLAY_TEXT
- GET_INKEY
- GET_INPUT
- SELECT_ITEM

Comment 17

[Carol Yang [:cyang]]

(In reply to Alexandre Poirot (:ochameau) from comment #15)
> (In reply to Carol Yang from comment #13)
> > However, is there anyway this solution can be more generic for other
> > proactive commands? There are other GCF TC which requires terminate session
> > which can benefit from sendSessionEndTROnFocusLose().
> 
> Is there any bug open for those other GCF TC tests failures? Or did you
> meant fixing all of those tests through this bug?

I had only opened this bug for the GET_INKEY case. If it makes sense, I can open another one that more generically tracks the other cases.

Comment 18

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

Comment on attachment 701117 [details]
Pull request 7553

I can give my approval but I think this bug is a blocking-tef+.

Let me ask to see.

Comment 19

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

https://github.com/mozilla-b2g/gaia/commit/be082e8b21c4b9228b69d3f0573f5118a03a544c

Not sure if this is the perfect UX but I guess we need to land that before the 15th...