STK Display text does not give option to terminate SIM session

RESOLVED FIXED

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cyang, Assigned: frsela)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

(Whiteboard: [cr 438074] QARegressExclude)

Attachments

(7 attachments)

(Reporter)

Description

5 years ago
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."
(Reporter)

Updated

5 years ago
Blocks: 808607
blocking-b2g: --- → tef+
frsela usually takes STK gaia bugs. frsela, can you take a look? thanks
Assignee: nobody → frsela
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.
Attachment #705856 - Flags: feedback?(cyang)
Created attachment 705858 [details]
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
Attachment #705858 - Flags: feedback?(dcoloma)
Flags: needinfo?(hello)
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.
Attachment #705858 - Flags: feedback?(dcoloma) → feedback+
(Reporter)

Comment 5

5 years ago
Created attachment 706192 [details]
UI from Comment 2 patch

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

Comment 6

5 years ago
(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?
(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
Attachment #705856 - Flags: review?(21)
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.
Attachment #705856 - Flags: review?(21) → review+
(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
Landed: https://github.com/mozilla-b2g/gaia/commit/6051da0e89ef6a669ffeab00acb22d89fc096e2d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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
status-b2g18: --- → fixed
(Reporter)

Comment 12

5 years ago
(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?
Status: RESOLVED → REOPENED
Flags: needinfo?(hello)
Resolution: FIXED → ---
(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.
Flags: needinfo?(cyang)

Comment 14

5 years ago
Created attachment 707059 [details]
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.
Created attachment 707079 [details]
Improved UI
Attachment #707079 - Flags: review?(21)
Created attachment 707080 [details]
Improved UI (screenshot)

Screenshot after 707079 patch
(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?
(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?
(Reporter)

Comment 19

5 years ago
(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.
Flags: needinfo?(cyang)
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
Attachment #707486 - Flags: review?(21)
Flags: needinfo?(cyang)
Attachment #707486 - Flags: review?(21) → review+
Attachment #707079 - Flags: review?(21) → review+
(Reporter)

Comment 21

5 years ago
(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!
Flags: needinfo?(cyang)
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
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
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?
status-b2g18: fixed → affected
status-b2g18-v1.0.0: --- → affected
v1-train (UI): 4c4123a88c2fac4591bf2574de36035a0faab033
v1-train (b): d9dd3c95e61714584ff63ffcb77f8622cbdd843a
v1.0.0 (UI): 69bf8afdf74a91d3f60838931522a0f6f081c9c5
v1.0.0 (b): 975eb799b957d048d2232021cd6b3d7aecd13fa1
status-b2g18: affected → fixed
status-b2g18-v1.0.0: affected → fixed
(Reporter)

Updated

5 years ago
Attachment #705856 - Flags: feedback?(cyang)

Updated

5 years ago
Whiteboard: [cr 438074] → [cr 438074] QARegressExclude

Comment 25

5 years ago
No need to create a TC in Moztrap for this defect.
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.