1166270 - It looks like that SMS services are sent before confirmation

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::SIM Tool Kit, defect, P2)

Description

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

STR:
1. have a SIM that has SMS services in the STK (Operator Services section in Settings). French Orange SIM has such services.
2. select a service that asks for a confirmation (examples on my SIM are "jokes" or "stock prices")
3. Do not press "OK"

=> I still get the answer from the service as a SMS. This makes me think the service was requested (and paid !) even than I didn't confirm. This is quite severe IMO.

Can QA test this on branches?

Comment 1

[Pi Wei Cheng [:piwei] (inactive)]

I was not able to reproduce this on Flame 3.0 or 2.2. Tested with a foreign SIM that has dictionary and a-word-a-day services. The a-word-a-day service requests service as soon as user taps on the button; I had mistaken this as reproducing the bug, but after a closer look it seems to be an expected behavior because the screen does tell me that the request has been made when I tap on the button even though I did not tap on OK. And the dictionary service does not request service until I hit OK.

Device: Flame 3.0
BuildID: 20150519010201
Gaia: 762cbd16712484f93f485e89f5363686540a3db7
Gecko: f65cc0022a0e
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Device: Flame 2.2
BuildID: 20150519002500
Gaia: 732acec6f37d13ccea6b0ddc48904a53a2970894
Gecko: 1389e6b8c065
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Leaving qawanted for others to attempt.

Comment 2

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Just to be sure: these were SMS services ? (I mean: did you receive a SMS ?)

Maybe we'd need videos to make sure we're talking about the same issue :)

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

Yes I did receive SMS's for these services. One service didn't need confirmation, and one needed confirmation and it did wait for my OK.

The foreign SIM is in Traditional Chinese so I doubt a video will make sense?

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I'd like a video if possible, I don't really mind about the actual text anyway :)

I'll do one myself tomorrow if I see something different. Maybe that's the different types of screen that do something different too.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

Here's a video.

https://www.youtube.com/watch?v=X5SlvvykEc4

First thing I did was to use the service that didn't need confirmation, note that I had to wait for a while for the SMS to arrive (around 35 seconds into the video I got the SMS), and it did say on screen that request is being processed. Then I went to the dictionary service and typed a word; I waited for a while making sure it's not processing what I typed, then I tapped OK, it says on screen the request is being processed and eventually SMS came in.

Comment 6

[Pi Wei Cheng [:piwei] (inactive)]

Julien, are you still able to reproduce this? If so, I don't think we have a SIM that could repro the bug hence we're going to exclude this bug in our queries.

Comment 7

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I'm sorry, I didn't have the time to look closer since then. I keep the NI but feel free to exclude the bug for now.

Comment 8

[Pi Wei Cheng [:piwei] (inactive)]

Thanks.

Comment 9

[Josh Cheng [:josh]]

[Blocking Requested - why for this release]:
Nominate to future release and continue investigation there.

Comment 10

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Basically the same behavior as what Pi Wei recorded in comment 5, albeit with a french SIM: https://www.youtube.com/watch?v=BmvMuyobnGM

So in my opinion we should not send the SMS request before the user taps on "OK".

With the current behavior:
* I don't understand how the 2 buttons Close/OK are different.
* When I tap on "Info" I don't know that a request will be sent (and thus that this will cost me money ?). This is especially unclear because other services wait for the OK (services with user input especially).

Pi Wei, in comment 1, you say "because the screen does tell me that the request has been made when I tap on the button", is it what's written in chinese? Would the chinese text be still right if we'd wait for the user pressing OK before sending the request ?
IMO the behavior is still wrong in chinese because you don't know a request will be sent _before_ the request is actually sent.

As you'll see on the video with the french SIM, it's really not clear a request has been sent.

Comment 11

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> Pi Wei, in comment 1, you say "because the screen does tell me that the
> request has been made when I tap on the button", is it what's written in
> chinese?

Yes. At 10 seconds into the video (https://www.youtube.com/watch?v=X5SlvvykEc4) the text under SIM 1 says "Processing, please wait".

> Would the chinese text be still right if we'd wait for the user
> pressing OK before sending the request ?

Mmm... If we were to change the user flow to the abovementioned design, it would make more sense if the button represents some sort of an action, such as a verb, like 'request'? Or some kind of text on the page that says press OK to send the request. For the current user flow the OK button seems redundant for this particular service.

> IMO the behavior is still wrong in chinese because you don't know a request
> will be sent _before_ the request is actually sent.

The user flow in Chinese makes sense to me, but I understand the need for any user to consciously confirm on any action coming up.

Comment 12

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Thanks, the chinese text makes sense. I'm not sure but I think the text itself comes from the carrier. Maybe that's why it's been implemented that way: the person who implemented this had such message coming from the carrier and it made sense for them.

NI UX to decide here.

Comment 13

[Tiffanie Shakespeare [:tif]]

Passing NI Harly for help on this!

Thanks for pinging the UX team!

Comment 14

[Harly Hsu[:harly]]

I agree with Julien's concerns in Comment 10. I think the message should not send out unless user presses the OK button. Also, for the text "OK", I am fine with changing it to "Request" in this case. Once the user presses the "Request" button, system will display a string like "Processing, please wait", and "Request" button will be disabled. Once the message has been received, the "Request" button will be enabled.

Comment 15

[Gregor Wagner [:gwagner]]

Yoshi, is this still your area?

Comment 16

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

Bevis is our hero here :D

Comment 17

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I'm quite sure the issue is in the Settings app, not in Gecko.

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] from comment #17)
> I'm quite sure the issue is in the Settings app, not in Gecko.

I agree.

According to the video, all the texts on the dialog and the menu items are directly communicated between System App(icc.js, icc_worker.js) and the STK Application in the Orange SIM card.

I am not pretty sure if that's the expected behavior to the STK App of the Orange SIM card.

To double confirm this, we can
1. capture logcat with ril & gaia debug flag enabled for further analysis.
2. compare the behavior of the same service with an non-fxos phone.

Comment 19

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Let's ask QA to capture this.

From what I understood in the STK code, we have different types of STK dialogs, and each dialog type has a specific behavior. And IMO the specific behavior for this specific dialog is wrong. That's this bug.

Now that UX agreed I think anybody could actually fix it.

Comment 20

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: RIL log

> 2. compare the behavior of the same service with an non-fxos phone.

QAnalysts can't do this because our SIM that supports STK is a regular sized SIM and can't be put into Aries which is our only device that runs on Android base. Julien will have to help on this.

Attached is a RIL log doing the STK service demonstrated at comment 5 video.

Tested on:
Device: Flame
BuildID: 20150818030206
Gaia: d9d99f32762975a370f1abd34a3512bd6fe29111
Gecko: 90d9b7c391d38ae118865bd87b5d011feee6dded
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5 Master) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 21

[Mahendra Potharaju [:mahe]]

Julien, Can you please see comment 20 above and help out? Thanks 

Making it a P2 for 2.5

Comment 22

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Actually only the 3rd Android phone I tried had the SIM Toolkit app.
I recorded the video here: https://youtu.be/bnIIrxQasq4

You'll see that the SMS request is sent as soon as we press the menu, and then we come back at the initial menu. We see the notification appearing at the top when we receive the SMS.

I still think it's better if we ask the user first, to match other types. What we have is close to Android but our confirmation screen that's not a confirmation is confusing.

Comment 23

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Hey Harly, just want to know if you still think this needs to be done with the additional information. (I still think it should !) :)

Comment 24

[Harly Hsu[:harly]]

I think there are 2 ways to deal with this issue:
1. Do not send out message unless user presses the "Request" button. Once the user presses the "Request" button, system will display a string like "Processing, please wait", and "Request" button will be disabled. Once the message has been received, the "Request" button will be enabled.

2. Send out message directly without any dialog or prompt and will get the message back from operator just like what Julien showed in the video.

Comment 25

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I'll have a look on this.

Comment 26

[GitHub Autolander Bot]

Attachment: [gaia] julienw:1166270-sms-services > mozilla-b2g:master

Comment 27

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8677550 [details] [review]
[gaia] julienw:1166270-sms-services > mozilla-b2g:master

I chose the option 2; ideally I'd have liked option 1 but it was really a too difficult option (if not impossible) given how STK works.

Hey Fernando, are you the right person to look at this? Is it appropriate to not display the result of the command at all? Maybe only on some conditions that I don't know?

Comment 28

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

(In reply to Julien Wajsberg [:julienw] from comment #27)
> Comment on attachment 8677550 [details] [review]
> [gaia] julienw:1166270-sms-services > mozilla-b2g:master
> 
> I chose the option 2; ideally I'd have liked option 1 but it was really a
> too difficult option (if not impossible) given how STK works.
> 
> Hey Fernando, are you the right person to look at this? Is it appropriate to
> not display the result of the command at all? Maybe only on some conditions
> that I don't know?

Hi,

The SMS is sent by the STK app inside the SIM card so you don't have any control to "abort" it.

I think the problem is not in our implementation but on the French Orange SIM that has such services since the STK app asks for a confirm message to the user and SHOULD act based on the user response but if this response is ignored, is the STK app who asks for confirmation (sending the options.text parameter) - I'll look for the official documentation about this

I case you want to ignore that message (options.text) the proposed patch LGTM

Comment 29

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Hey Fernando,

I can wait if you want to look at the official documentation; I can also have a look by myself if you can point me to the right direction :) I found [1] but it's quite difficult to read.
[1] http://www.3gpp.org/DynaReport/31111.htm

Also what puzzles me is the behavior in Android where they don't wait for a confirmation, that's why I implemented basically the same (except they output a notice at the bottom).

I'd like a final stamp from your side before merging :)

Comment 30

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8677550 [details] [review]
[gaia] julienw:1166270-sms-services > mozilla-b2g:master

I realize we should display the received text in a banner, just like android does. Will provide a new patch today.

Comment 31

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8677550 [details] [review]
[gaia] julienw:1166270-sms-services > mozilla-b2g:master

Hey Fernando Jimenez,

I wonder if you could review this.

Frsela reviewed an earlier version of the patch (that did a lot less things -- mainly removed most of the behavior). In this new patch, I added some behavior back, showing a System Banner instead of a "confirm" or "alert" dialog. It's a lot closer to what happens on Android and it makes quite sense, at least with the services I have on my Orange SIM.

I did some small changes to SystemBanner as well, to accept the "{raw: XXX}" syntax for the l10n arguments, with added unit tests :)

Hey Fernando Sela, please feel free to chime in as well !

Hey Pi Wei, I'd appreciate if you can try this patch with your chinese SIM and report how this feels.

Thanks to all !

Comment 32

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8677550 [details] [review]
[gaia] julienw:1166270-sms-services > mozilla-b2g:master

The code LGTM, but I really have no idea about STK, so you'll need Fernando Selas' rubber stamp.

Comment 33

[Pi Wei Cheng [:piwei] (inactive)]

Comment on attachment 8677550 [details] [review]
[gaia] julienw:1166270-sms-services > mozilla-b2g:master

Three problems.

1) The flow of UI seems odd. Before the patch, say I need to go through menu A > menu B > menu C, on menu C I can make a request to STK, and after that I tap back arrow button or OK button, and it would briefly show menu C and then takes me to menu A.

After patch, as soon as I make a request on menu C, it auto takes me back to menu A.

The original flow is odd, because the back button doesn't take me back to menu C, but at least I know what I was doing before I tap back/ok. Now as soon as the request is made it goes back to menu A.

2) Not sure if this patch made this bug more visible but after several requests it seems to fail on making subsequent STK requests and renders all related menu options not usable (tapping on them does nothing). I will attach a RIL logcat of this particular bug.

3) I could have missed conversations above, but still there's no confirmation on certain STK requests, which is what this bug is originally about?

Comment 34

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: RIL log for comment 33 issue no. 2

Attaching log for issue observed at comment 33 problem no. 2.

Comment 35

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

(In reply to Pi Wei Cheng [:piwei] from comment #33)
> Comment on attachment 8677550 [details] [review]
> [gaia] julienw:1166270-sms-services > mozilla-b2g:master
> 
> Three problems.
> 
> 1) The flow of UI seems odd. Before the patch, say I need to go through menu
> A > menu B > menu C, on menu C I can make a request to STK, and after that I
> tap back arrow button or OK button, and it would briefly show menu C and
> then takes me to menu A.
> 
> After patch, as soon as I make a request on menu C, it auto takes me back to
> menu A.
> 
> The original flow is odd, because the back button doesn't take me back to
> menu C, but at least I know what I was doing before I tap back/ok. Now as
> soon as the request is made it goes back to menu A.

This is the same behavior as on Android (see https://youtu.be/bnIIrxQasq4). I agree with you this feels odd, but I can't help thinking this is how STK works :/

I can try another behavior though: instead of showing the System banner, still show an "alert" (instead of the "confirm" we had before).

> 
> 2) Not sure if this patch made this bug more visible but after several
> requests it seems to fail on making subsequent STK requests and renders all
> related menu options not usable (tapping on them does nothing). I will
> attach a RIL logcat of this particular bug.

I've seen it as well, but I'm quite sure it's the case for some time already. I'd appreciate if you can try on master as well ?

> 
> 3) I could have missed conversations above, but still there's no
> confirmation on certain STK requests, which is what this bug is originally
> about?

Yes, see comment 28:
> The SMS is sent by the STK app inside the SIM card so you don't have any control to
> "abort" it.

So we can't have a confirmation before it's sent, it's just not technically doable given how STK works :(


I'll provide another patch so that UX can try the 2 behaviors and chose.

Comment 36

[GitHub Autolander Bot]

Attachment: [gaia] julienw:1166270-sms-services-2 > mozilla-b2g:master

Comment 37

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

OK I realize there is actually something wrong with the first patch. Thanks Pi-Wei for finding it. I think I need to properly terminate the response.

Comment 38

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

removing requests until I sort this out.

Comment 39

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8681212 [details] [review]
[gaia] julienw:1166270-sms-services-2 > mozilla-b2g:master

Hey Fernando,

so here is another proposal. This time I'm using alert instead of nothing (first proposal) or system banner (second proposal). For an unknown reason (I coudn't find why) the previous proposal can break some use cases. Some STK requests were just not working anymore.

With this new proposal everything seems to work better. The change is also smaller.

See also https://youtu.be/TtqmwLYSgTg for the new behavior. asking ui-review from Harly for this change. 

(As a reminder, see https://youtu.be/aR50IZNk4Ao for the previous behavior doing exactly the same actions).

Comment 40

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

(In reply to Julien Wajsberg [:julienw] from comment #37)
> OK I realize there is actually something wrong with the first patch. Thanks
> Pi-Wei for finding it. I think I need to properly terminate the response.

Fernando, if you find time to look at this video as well: https://youtu.be/tSmZLZ_4xXY
This is with the patch in attachment 8677550 [details] [review], and you can see the second request does not work. (however if I do several times the first request it works properly). I wonder if you have some idea about this.

Comment 41

[Pi Wei Cheng [:piwei] (inactive)]

Comment on attachment 8681212 [details] [review]
[gaia] julienw:1166270-sms-services-2 > mozilla-b2g:master

The UI flow seems better (to me at least) in this patch. I also didn't encounter the STK requests stopped sending and menus nonfunctional afterwards issue.

However there's one issue I observed in this patch. I'm going to write STR since I don't have time to do a video today.

STR:
1) Proceed to make an STK request
2) Do NOT hit the 'Close' button after you have made the request. Instead wait on this page until you've received an SMS notification
3) Tap via utility tray to display the notification

Observe that the STK request page at step 2 is obscuring the Messages app after the transition.

Other than the above issue I didn't observe anything else that stand out.

Comment 42

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Pi Wei Cheng [:piwei] from comment #41)
> 3) Tap via utility tray to display the notification

It should be to display the message, not notification. Tap on notification to go to the message.

Comment 43

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Mmm I think this is also happening on master, but I'll double check. If yes I agree this should be fixed but in a separate bug, and I think someone from the SystemFE team will know better than me :)

Comment 44

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

(In reply to Julien Wajsberg [:julienw] from comment #43)
> Mmm I think this is also happening on master, but I'll double check. If yes
> I agree this should be fixed but in a separate bug, and I think someone from
> the SystemFE team will know better than me :)

Yes, same on master. I filed bug 1220552 to handle this separate issue.

Comment 45

[Harly Hsu[:harly]]

Hi Julien,
The patch looks good to me. Just wondering if is it possible to change the OK button to "Request" or "Send"?

Comment 46

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

(In reply to Harly Hsu[:harly] from comment #45)
> Hi Julien,
> The patch looks good to me. Just wondering if is it possible to change the
> OK button to "Request" or "Send"?

Hey Harly,

which button ?

If it's in the final "alert" window, then it's too late, the SMS request was already sent at this stage. As we discussed earlier, given how STK works, we can't have an alert or confirm window before the request is sent. So the patch here is only using an 'alert' type because the user can't have any meaningful action.

If it's in the previous window (example for Horoscope here) then why not, but I'd prefer to do do it in a separate bug. Note we don't have the previous window in all cases (example of "Info" in my video).

Comment 47

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

hey Tim, do you think you could find somebody to review this ? This part is quite unowned now that TEF does not develop anymore :/

Comment 48

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

... r=me :P and it's yours? We don't really have the test coverage for this part of code to be honest.

If you don't feel comfortable doing that I can r+ here, but don't trust my review...

Comment 49

[Harly Hsu[:harly]]

Yes, I mean the previous window. If you will open another bug to change the OK button, then I will give r+ for the ui-review for this bug in this case. Thanks

Comment 50

[Harly Hsu[:harly]]

Comment on attachment 8681212 [details] [review]
[gaia] julienw:1166270-sms-services-2 > mozilla-b2g:master

LGTM :)

Comment 51

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Thanks Harly, I filed bug 1220586.

Tim, I don't really want to own this :p

Comment 52

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Comment on attachment 8681212 [details] [review]
[gaia] julienw:1166270-sms-services-2 > mozilla-b2g:master

Wow I totally forgot about it.

Tim, I'd appreciate your review, or you can redirect to somebody else if you want (maybe Fred ?).

Comment 53

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

Thanks !

master: 011476bde4084feee4b6364a98baa792227a4825

Comment 54

[Julien Wajsberg [:julienw] (PTO -> Aug 24)]

I don't think this is serious enough to warrant an uplift, and this part of the code is not well covered in tests. So let's remove the nomination.