833630 - STK Display text does not give option to terminate SIM session

Status: RESOLVED FIXED

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

Description

[Carol Yang [:cyang]]

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

- Start GCF test which prompts for device power up
- After power up, GCF test will send a proactive command for DISPLAY_TEXT

At this point, a popup with "<ABORT>" message is displayed with Back and OK buttons underneath. However, there are no options to terminate the SIM session, causing this GCF test to fail.

There should be an option to terminate the session according to TS 11.14, section 6.4.1:

"If the user has indicated the need to end the proactive SIM application session, the ME shall send a TERMINAL RESPONSE with "Proactive SIM application session terminated by the user" result value."

Comment 1

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

frsela usually takes STK gaia bugs. frsela, can you take a look? thanks

Comment 2

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

Attachment: Added a STK close button to DISPLAY TEXT

Carol, this is only to check if this patch passes the GCF test.

We're working on a better UI experience with this.

Comment 3

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

Attachment: Current UI with 3 buttons

Dani, as we talked before, the close button is over the others.

I think the OK button should be the first one or show the close as an X in a top corner

Comment 4

[Daniel Coloma:dcoloma]

Comment on attachment 705858 [details]
Current UI with 3 buttons

It is not Very nice, but at this moment the priority is passanen the GCF test.

Comment 5

[Carol Yang [:cyang]]

Attachment: UI from Comment 2 patch

This is the UI seen after I applied the patch from attachment 705856 [details].

Comment 6

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #2)
> Created attachment 705856 [details]
> Added a STK close button to DISPLAY TEXT
> 
> Carol, this is only to check if this patch passes the GCF test.
> 
> We're working on a better UI experience with this.

Hi Fernando, I'm not sure if it's because I'm missing something but picking up the changes from Comment 2, I don't see what you see in Comment 3. Took a screen shot (attached). It's just the changes in the two files (icc.js and index.html), right?

Comment 7

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

(In reply to Carol Yang from comment #6)
> (In reply to Fernando R. Sela [:frsela] from comment #2)
> > Created attachment 705856 [details]
> > Added a STK close button to DISPLAY TEXT
> > 
> > Carol, this is only to check if this patch passes the GCF test.
> > 
> > We're working on a better UI experience with this.
> 
> Hi Fernando, I'm not sure if it's because I'm missing something but picking
> up the changes from Comment 2, I don't see what you see in Comment 3. Took a
> screen shot (attached). It's just the changes in the two files (icc.js and
> index.html), right?

Ups. I didn't uploaded the last version. In your case, the left one button is the close one.

You only need to change the index.html file to fix this, adding this line inside the section area:

<button data-l10n-id="close" id="icc-stk-alert-btn_close">Close</button>

I just uploaded the latest one to github. sorry for the inconvenience

Comment 8

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

Comment on attachment 705856 [details]
Added a STK close button to DISPLAY TEXT

I do wonder why you need to add 'settings' as a parameter to app.launch. If there isn't any reasons let's remove it.

Comment 9

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

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #8)
> Comment on attachment 705856 [details]
> Added a STK close button to DISPLAY TEXT
> 
> I do wonder why you need to add 'settings' as a parameter to app.launch. If
> there isn't any reasons let's remove it.

I think you mean on another path, this one don't use app.launch

Comment 10

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

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

Comment 11

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

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

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

Comment 12

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #10)
> Landed:
> https://github.com/mozilla-b2g/gaia/commit/
> 6051da0e89ef6a669ffeab00acb22d89fc096e2d

Hi Fernando,

The UI with this patch looks the same as what you had shown before. However, pressing the Close button only dismisses the DISPLAY_TEXT. I do not see "RIL:SendStkResponse" with resultCode STK_RESULT_UICC_SESSION_TERM_BY_USER. For the other buttons (OK and Back), pressing them will issue the corresponding resultCode. When you're testing, do you see the resultCode for STK_RESULT_UICC_SESSION_TERM_BY_USER?

Comment 13

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

(In reply to Carol Yang from comment #12)
> (In reply to Fernando R. Sela [:frsela] from comment #10)
> > Landed:
> > https://github.com/mozilla-b2g/gaia/commit/
> > 6051da0e89ef6a669ffeab00acb22d89fc096e2d
> 
> Hi Fernando,
> 
> The UI with this patch looks the same as what you had shown before. However,
> pressing the Close button only dismisses the DISPLAY_TEXT. I do not see
> "RIL:SendStkResponse" with resultCode STK_RESULT_UICC_SESSION_TERM_BY_USER.
> For the other buttons (OK and Back), pressing them will issue the
> corresponding resultCode. When you're testing, do you see the resultCode for
> STK_RESULT_UICC_SESSION_TERM_BY_USER?

Hi Carol, I don't understand, I reviewed the merged patch (https://github.com/mozilla-b2g/gaia/pull/7781/files) and the close button is as the https://bugzilla.mozilla.org/attachment.cgi?id=705858 screenshot.

Also, when CLOSE button is pressed the STK_RESULT_UICC_SESSION_TERM_BY_USER (https://github.com/mozilla-b2g/gaia/pull/7781/files#L1R77) should be sent to the SIM card.

Can you re-check again with the last Gaia version? since this patch is landed.

Comment 14

[Rafael Rebolleda]

Attachment: Three button UI example

Here's an example of what three buttons alongside might look like (although we ended up not using that particular feature for other reasons).

I think this approach would be much better than the current one.

Comment 15

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

Attachment: Improved UI

Comment 16

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

Attachment: Improved UI (screenshot)

Screenshot after 707079 patch

Comment 17

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

(In reply to Fernando R. Sela [:frsela] from comment #16)
> Created attachment 707080 [details]
> Improved UI (screenshot)
> 
> Screenshot after 707079 patch

It seems an horrible UI. Is it really what UX want?

Comment 18

[Daniel Coloma:dcoloma]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #17)
> (In reply to Fernando R. Sela [:frsela] from comment #16)
> > Created attachment 707080 [details]
> > Improved UI (screenshot)
> > 
> > Screenshot after 707079 patch
> 
> It seems an horrible UI. Is it really what UX want?

Quite ugly, I can only agree, but this is a cornercase and at this stage I only care here about passing GCF tests our partners require. 

Vivien, is the code OK?
Carol, can you confirm the patch would make GCF test pass?

Comment 19

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #13)
> (In reply to Carol Yang from comment #12)
> > (In reply to Fernando R. Sela [:frsela] from comment #10)
> > > Landed:
> > > https://github.com/mozilla-b2g/gaia/commit/
> > > 6051da0e89ef6a669ffeab00acb22d89fc096e2d
> > 
> > Hi Fernando,
> > 
> > The UI with this patch looks the same as what you had shown before. However,
> > pressing the Close button only dismisses the DISPLAY_TEXT. I do not see
> > "RIL:SendStkResponse" with resultCode STK_RESULT_UICC_SESSION_TERM_BY_USER.
> > For the other buttons (OK and Back), pressing them will issue the
> > corresponding resultCode. When you're testing, do you see the resultCode for
> > STK_RESULT_UICC_SESSION_TERM_BY_USER?
> 
> Hi Carol, I don't understand, I reviewed the merged patch
> (https://github.com/mozilla-b2g/gaia/pull/7781/files) and the close button
> is as the https://bugzilla.mozilla.org/attachment.cgi?id=705858 screenshot.
> 

Hi Fernando, Perhaps I wasn't being clear enough. I do see the UI the same way you shared in the screenshot, so no problems with the UI matching yours.

> Also, when CLOSE button is pressed the STK_RESULT_UICC_SESSION_TERM_BY_USER
> (https://github.com/mozilla-b2g/gaia/pull/7781/files#L1R77) should be sent
> to the SIM card.
> 
> Can you re-check again with the last Gaia version? since this patch is
> landed.

I retested with latest Gaia version which has this patch landed. I got the same result that I saw on 1/25. Pressing the "Close" button only dismisses the DISPLAY_TEXT. I do not see STK_RESULT_UICC_SESSION_TERM_BY_USER get sent. Can you please test on your end to see if you do? As of now, the GCF TC will fail if terminate session by user 0x10 is not sent.

Comment 20

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

Attachment: Fix comment #19 -> Send STK close response. (Activate commandprocessed flag)

Also on close & back stops timer to avoid two STK responses.

@carol, Could you check if this patch fix the issue commented on #19

Comment 21

[Carol Yang [:cyang]]

(In reply to Fernando R. Sela [:frsela] from comment #20)
> Created attachment 707486 [details]
> Fix comment #19 -> Send STK close response. (Activate commandprocessed flag)
> 
> Also on close & back stops timer to avoid two STK responses.
> 
> @carol, Could you check if this patch fix the issue commented on #19

Hi Fernando, This patch will issue 0x10 to indicate terminate session. Looks good! Thanks!

Comment 22

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

Thanks Carol & Vivien.

Both patches landed.

UI: https://github.com/mozilla-b2g/gaia/commit/7ab9a451b737c7368263e79a5bc6e5219f65f34a

Close fix: https://github.com/mozilla-b2g/gaia/commit/6701b194729bd8031c6e04d210898bf2205f3a42

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

Moving status flags back to affected since there are new commits for this issue in comment 22 - can someone confirm that those get uplifted to v1-train and v1.0.0 branches?

Comment 24

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

v1-train (UI): 4c4123a88c2fac4591bf2574de36035a0faab033
v1-train (b): d9dd3c95e61714584ff63ffcb77f8622cbdd843a
v1.0.0 (UI): 69bf8afdf74a91d3f60838931522a0f6f081c9c5
v1.0.0 (b): 975eb799b957d048d2232021cd6b3d7aecd13fa1

Comment 25

[amiller]

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