1046564 - [tarako/dolphin] Should display normal priority message of STK_CMD_DISPLAY_TEXT command while device is Settings

Status: RESOLVED FIXED

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

Description

[helloarvin]

Attachment: dolphin-stk-issue-logs

Steps to reproduce:
1> Boot the dolphin with sim card inserted;
2> enter settings app and click STK menu;
3> click several items and there's no expected content shown to user（e.g. CMCC）
"业务推荐"
“SIM卡信息”--“卡类信息查看”/“卡剩余空间查看”/“OTA功能介绍”

Analysis result:
An StkTerminalResponse with the resultCode 32 will be sent immediately the message 'RIL:StkCommand' received, and this response will lead modem report the current page content so that there's no changes in UI.

I have two questions on this:
a) AFAIK, commandQualifier 128(0X80) stands for 'wait user to clear the message' and 0 means 'clear message after time delay' according to the protocol. Why we send the response immediately the message received?

b) Who send the response with the resultCode 32, gaia or gecko?

--------------------------------------------------------------------

// click the item '卡类信息查看' and an AT cmd will be sent
07-30 11:30:12.280 D/use-Rlog/RLOG-AT(  129): [w] Channel4: AT> AT+SPUSATTERMINAL="810301240002028281830100900101"

// result from modem 
07-30 11:30:12.320 D/use-Rlog/RLOG-AT(  129): [w] Channel3: AT< +SPUSATPROCMDIND:D0348103012180820281028D2908672C53614E3A00360034004B002000530049004D5361FF0C652F6301004F00540041529F80FD3002

// RILC parse the results
07-30 11:30:12.320 I/Gecko   (  108): RIL Worker: [1] Unsolicited response for request type 1013
07-30 11:30:12.320 I/Gecko   (  108): RIL Worker: [1] Handling parcel as UNSOLICITED_STK_PROACTIVE_COMMAND
07-30 11:30:12.350 I/Gecko   (  108): RIL Worker: [1] Read UCS2 string: 本卡为64K SIM卡，支持OTA功能。
07-30 11:30:12.350 I/Gecko   (  108): RIL Worker: [1] commandNumber = 1 typeOfCommand = 21 commandQualifier = 128
07-30 11:30:12.350 I/Gecko   (  108): -*- RadioInterface[1]: Received message from worker: {"commandNumber":1,"typeOfCommand":33,"commandQualifier":128,"rilMessageType":"stkcommand","options":{"text":"本卡为64K SIM卡，支持OTA功能。","userClear":true},"rilMessageClientId":1}
07-30 11:30:12.350 I/Gecko   (  108): -*- RadioInterface[1]: handleStkProactiveCommand {"commandNumber":1,"typeOfCommand":33,"commandQualifier":128,"rilMessageType":"stkcommand","options":{"text":"本卡为64K SIM卡，支持OTA功能。","userClear":true},"rilMessageClientId":1}
07-30 11:30:12.360 I/Gecko   (  108): RIL Worker: Next parcel size unknown, going to sleep.
07-30 11:30:12.380 I/Gecko   (  615): -*- RILContentHelper: Received message 'RIL:StkCommand': {"clientId":1,"data":{"commandNumber":1,"typeOfCommand":33,"commandQualifier":128,"rilMessageType":"stkcommand","options":{"text":"本卡为64K SIM卡，支持OTA功能。","userClear":true},"rilMessageClientId":1}}
07-30 11:30:12.390 I/Gecko   (  108): -*- RILContentHelper: Received message 'RIL:StkCommand': {"clientId":1,"data":{"commandNumber":1,"typeOfCommand":33,"commandQualifier":128,"rilMessageType":"stkcommand","options":{"text":"本卡为64K SIM卡，支持OTA功能。","userClear":true},"rilMessageClientId":1}}


// KEY --- send StkResponse here with "resultCode":32

07-30 11:30:12.390 I/Gecko   (  108): -*- RadioInterfaceLayer: Received 'RIL:SendStkResponse' message from content process
07-30 11:30:12.390 I/Gecko   (  108): RIL Worker: [1] Received chrome message {"resultCode":32,"additionalInformation":1,"command":{"commandNumber":1,"typeOfCommand":33,"commandQualifier":128,"rilMessageType":"stkcommand","options":{"text":"本卡为64K SIM卡，支持OTA功能。","userClear":true},"rilMessageClientId":1},"rilMessageClientId":1,"rilMessageToken":27,"rilMessageType":"sendStkTerminalResponse"}

// to send AT command
07-30 11:30:12.410 D/use-Rlog/RLOG-AT(  129): [w] Channel4: AT> AT+SPUSATTERMINAL="810301218002028281830120"

// request result from modem
07-30 11:30:12.450 D/use-Rlog/RLOG-AT(  129): [w] Channel3: AT< +SPUSATPROCMDIND:D03D8103012400820281828F0E018053617C7B4FE1606F67E5770B8F100280536152694F597A7A95F467E5770B8F100380004F00540041529F80FD4ECB7ECD

// RILC parsed the result and the content was the current page info
07-30 11:30:12.460 I/Gecko   (  108): RIL Worker: [1] Handling parcel as UNSOLICITED_STK_PROACTIVE_COMMAND
07-30 11:30:12.460 I/Gecko   (  108): RIL Worker: [1] commandNumber = 1 typeOfCommand = 24 commandQualifier = 0
07-30 11:30:12.470 I/Gecko   (  108): -*- RadioInterface[1]: Received message from worker: {"commandNumber":1,"typeOfCommand":36,"commandQualifier":0,"rilMessageType":"stkcommand","options":{"items":[{"identifier":1,"text":"卡类信息查看"},{"identifier":2,"text":"卡剩余空间查看"},{"identifier":3,"text":"OTA功能介绍"}],"presentationType":0},"rilMessageClientId":1}

Comment 1

[helloarvin]

Hi Hsin-Yi,

I've tried to research the code and log since yesterday but I'm still in a state of confusion. Could you please help check my analysis and confirm the root-cause of the issue?

Thank you very much.

Comment 2

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Hi helloarvin:

Please see my below comment.

(In reply to helloarvin from comment #0)
> Created attachment 8465254 [details]
> dolphin-stk-issue-logs
> 
> Steps to reproduce:
> 1> Boot the dolphin with sim card inserted;
> 2> enter settings app and click STK menu;
> 3> click several items and there's no expected content shown to user（e.g.
> CMCC）
> "业务推荐"
> “SIM卡信息”--“卡类信息查看”/“卡剩余空间查看”/“OTA功能介绍”
> 
> Analysis result:
> An StkTerminalResponse with the resultCode 32 will be sent immediately the
> message 'RIL:StkCommand' received, and this response will lead modem report
> the current page content so that there's no changes in UI.
> 
> I have two questions on this:
> a) AFAIK, commandQualifier 128(0X80) stands for 'wait user to clear the
> message' and 0 means 'clear message after time delay' according to the
> protocol. Why we send the response immediately the message received?

STK_CMD_DISPLAY_TEXT is handled by gaia [1]. Please see https://github.com/mozilla-b2g/gaia/blob/6d6291e8e11f459f2cc36acc98ab845fa78abe47/apps/system/js/icc_worker.js#L226-L270

> 
> b) Who send the response with the resultCode 32, gaia or gecko?

I guess the resultCode 32 is from https://github.com/mozilla-b2g/gaia/blob/6d6291e8e11f459f2cc36acc98ab845fa78abe47/apps/system/js/icc_worker.js#L233-L241

Comment 3

[helloarvin]

Hi edgar,

Thank you very much for your kindly help, I appreciate it.


Hi Fernando,

I think we should modify the judgement condition of https://github.com/mozilla-b2g/gaia/blob/6d6291e8e11f459f2cc36acc98ab845fa78abe47/apps/system/js/icc_worker.js#L233
Because the text with normal priority will never be shown even if the current panel is STK, dose this case belong to 'screen busy'?

Could you please help check the issue and feel free to contact me if there's something wrong.

Thanks.

Comment 4

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

(In reply to helloarvin from comment #3)
> Hi edgar,
> 
> Thank you very much for your kindly help, I appreciate it.
> 
> 
> Hi Fernando,
> 
> I think we should modify the judgement condition of
> https://github.com/mozilla-b2g/gaia/blob/
> 6d6291e8e11f459f2cc36acc98ab845fa78abe47/apps/system/js/icc_worker.js#L233
> Because the text with normal priority will never be shown even if the
> current panel is STK, dose this case belong to 'screen busy'?
> 
> Could you please help check the issue and feel free to contact me if there's
> something wrong.
> 
> Thanks.

Current implementation will show the message only if the device is in the HomeScreen.

If you are browsing in Settings app, the message won't be show which is a mistake.

So we should check if the active app is HomeScreen or Settings.

  AppWindowManager.getActiveApp().origin === 'app://settings.gaiamobile.org'

or check if exists settings

  AppWindowManager.getApp('app://settings.gaiamobile.org')

Will this fix your issue?

Comment 5

[helloarvin]

Yeah, I prefer the former solution[1] for we just need show the message if current app is HomeScreen or Settings. Do you think so?

Thanks.

[1]AppWindowManager.getActiveApp().origin === 'app://settings.gaiamobile.org'

Comment 6

[helloarvin]

Attachment: PR for the branch v1.4

Hi Fernando,

I've created a PR based on your suggestion. Could you please help review it?

Thanks a lot.

Comment 7

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

Comment on attachment 8466973 [details] [review]
PR for the branch v1.4

r+ with a NIT (the comment was an idea, but it's better avoid magic values. The URL can be changed in the future, so better avoid use it directly.

can you propose another patch for 1.3?

Take into account that is a little different: [1]

[1] https://bug1015833.bugzilla.mozilla.org/attachment.cgi?id=8465999

Thank you !

Comment 8

[helloarvin]

Hi Fernando,

I've modified the PR[1] based on your beneficial suggestion. Please help landed it into the branch v1.4 if there's nothing needs to be improved.

After the patch[2] merged, I'll create another patch for the issue for v1.3t asap.

Thanks.

[1] https://github.com/mozilla-b2g/gaia/pull/22478 
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1015833

Comment 9

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

Comment on attachment 8466973 [details] [review]
PR for the branch v1.4

Asking review to a System owner.

Comment 10

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

(In reply to helloarvin from comment #8)
> Hi Fernando,
> 
> I've modified the PR[1] based on your beneficial suggestion. Please help
> landed it into the branch v1.4 if there's nothing needs to be improved.
> 
> After the patch[2] merged, I'll create another patch for the issue for v1.3t
> asap.
> 
> Thanks.
> 
> [1] https://github.com/mozilla-b2g/gaia/pull/22478 
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1015833

I'll help you ;)
Thank you for your work :)

Comment 11

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

[Blocking Requested - why for this release]: without this patch, non priority messages STK won't be show if settings is active. Sometimes these messages comes as a response to a user action in the settings app so this patch allows STK messages over system and settings.

Comment 12

[Wayne Chang [:wchang]]

per comment 11 - user impact may be loss of STK messages

Comment 13

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

Landed in 1.4 branch: https://github.com/mozilla-b2g/gaia/commit/e9dce1f60f729e228810f751417681b5ff937b6b

Comment 14

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

(In reply to helloarvin from comment #8)
> 
> After the patch[2] merged, I'll create another patch for the issue for v1.3t
> asap.
> 
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1015833


Hi,

If you can rebase to 1.3t and also master (for 2.0 and 2.1) will be great.

Thanks in advance

Comment 15

[helloarvin]

Attachment: PR for master

Hi Fernando & Alive,

I've created the PR for master and please help review it.
BTW, the PR for the branch v1.3t will be available until the patch[1] merged.(So I'd like to keep ni state here)

Thanks.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1015833

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Bug resolution tracks landing on master. Let's leave this bug open until then. Also, please note for the future that we typically require patches to land on master *before* they land on the release branches.

Also, if this needs to land on v2.0, you'll need to request approval on the patch to do so since 1.4+ doesn't grant auto-approval anymore. Sorry for the inconvenience.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8468399 [details] [diff] [review]
PR for master

r+ wit nit

Comment 18

[helloarvin]

Hi alive,

Thanks a lot for your kind advice.
I've modified the PR[1] and please help check and merge into master :)

[1] https://github.com/arvin-zhang/gaia/commit/973868385cceb3f48ca79b6492b55446b4ec3168

Comment 19

[helloarvin]

Attachment: PR for the branch v1.3t

Hi alive,

I've created the PR for v1.3t since the blocker bug1015833 merged just now. Please help review it, thanks.

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8469079 [details] [review]
PR for the branch v1.3t

r- for totally wrong origin.

Comment 21

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

Comment on attachment 8469079 [details] [review]
PR for the branch v1.3t

Hi Alive,

Arvin fixed the patch [1]. Can you review it again.

Thanks in advance

[1] https://github.com/mozilla-b2g/gaia/pull/22614#issuecomment-51738611

Comment 22

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8469079 [details] [review]
PR for the branch v1.3t

Please test it next time you write a patch.

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8468399 [details] [diff] [review]
PR for master

Please test it next time you have a patch.

Comment 24

[helloarvin]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #23)
> Comment on attachment 8468399 [details] [diff] [review]
> PR for master
> 
> Please test it next time you have a patch.

Got it.
Sorry for bringing you too much trouble.

Comment 25

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

Landed:

Master: https://github.com/mozilla-b2g/gaia/commit/57dd7aa9c746dcf52d9c09d149be69eaa497e8a7
v1.3t: https://github.com/mozilla-b2g/gaia/commit/e429aaba09f9f29a3ea82811e483b82fa92a6556

Comment 26

[helloarvin]

Hi Fernando,
Landed of v1.4 : https://github.com/mozilla-b2g/gaia/commit/e9dce1f60f729e228810f751417681b5ff937b6b

it is:
+    var settingsOrigin = document.location.protocol + '//' +
+      document.location.host.replace('system', 'settings');
not :
+    var settingsOrigin = window.location.origin.replace('system', 'settings');

Should I create a new PR to update for v1.4, thanks?

Comment 27

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

Both methods works. I'm not really sure if it's needed to update in 1.4 since it's landed

Comment 28

[(inactive after 6/18) Alive Kuo [:alive]]

Please backout the original one or quickly fix it.

Comment 29

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

Reverted in 1.3T: https://github.com/mozilla-b2g/gaia/commit/c2790220c2ed9acacd6a11d31409e7807cf7afa0

I forgot asking for approval. Sorry for the inconveniences

Comment 30

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

Hi Arvin,

I forgot to ask for an approval to landing in 1.3T. You need to ask approval‑gaia‑v1.3? in the patch details an fill the risks section.

As soon as you have 1.3+ I'll merge your changes again.

Comment 31

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

Arvin, the same for landing in 2.0 (but asking approval in the master patch)

Comment 32

[helloarvin]

Comment on attachment 8469079 [details] [review]
PR for the branch v1.3t

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):feature request(see comment 4)
[User impact] if declined:The STK TEXT message with normal priority won't be shown if device is in Settings app.
[Testing completed]:self test passed
[Risk to taking this patch] (and alternatives if risky):not at all
[String changes made]:none

Comment 33

[helloarvin]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #28)
> Please backout the original one or quickly fix it.

Hi Alive,

The patch already landed into v1.4 works fine. I think it is the 'original one' mentioned above rather than the wrong version of combining 'document' and 'window'.
+    var settingsOrigin = document.location.protocol + '//' +
+      document.location.host.replace('system', 'settings');

So, Let's keep it, thanks.

Comment 34

[helloarvin]

Comment on attachment 8468399 [details] [diff] [review]
PR for master

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):feature request(see comment 4)
[User impact] if declined:The STK TEXT message with normal priority won't be shown if device is in Settings app.
[Testing completed]:self test passed
[Risk to taking this patch] (and alternatives if risky):not at all
[String changes made]:none

Comment 35

[bhavana bajaj [:bajaj]]

frsela, is this a regression  in 1.3 or 1,3 ? Or is this a long standing issue we are trying to fix? If its the latter I would like this to ride the trains as it would not block the 2.0 release.

Comment 36

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

1.3t landed again ;)

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

Comment 37

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

(In reply to bhavana bajaj [:bajaj] from comment #35)
> frsela, is this a regression  in 1.3 or 1,3 ? Or is this a long standing
> issue we are trying to fix? If its the latter I would like this to ride the
> trains as it would not block the 2.0 release.

Hi,

No regression, no new feature.
This is fixing an issue with non-prioritary STK messages. Not sure if this answer your question

Comment 38

[Yeojin Chung [:YeojinC]]

New test case needs to be added. There is no existing test case.

Comment 39

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14340/

Comment 40

[bhavana bajaj [:bajaj]]

marking 2.1 fixed as this seems to have landed in master.

Comment 41

[bhavana bajaj [:bajaj]]

Comment on attachment 8468399 [details] [diff] [review]
PR for master

very low risk STK change.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/54ec8ac1e57b3d885e2d4afd10a9de79247b9429

Comment 43

[Paladin]

This issue has been verified successfully on Flame 2.0,2.1

See attachment: Verify_video.3gp
Reproducing rate: 0/5
Flame 2.0 build:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141207000206
Version         32.0
Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0

Comment 44

[Paladin]

Attachment: Verify_video.3gp