827860 - B2G STK: Ril throws and get stuck when default text is null

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Alexandre Poirot [:ochameau]]

While testing bug 827655, I hit that exception and ended with a locked STK menu when trying to enter "Recherche" > "2easy" menu of vivien's orange simcard.

http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#7396
Here, `ctlv.value` is undefined as ctlv looks like this:
{"tag":23,"length":0,"value":null,"cr":false,"hlen":2}

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 827860 - STK Ril throws and get stuck when default string is empty

Comment 2

[Alexandre Poirot [:ochameau]]

Nomitating as STK is heavily tested during certification and we may hit that exception.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 699236 [details] [diff] [review]
Bug 827860 - STK Ril throws and get stuck when default string is empty

Review of attachment 699236 [details] [diff] [review]:
-----------------------------------------------------------------

Edgar will help attaching a test case for this. :)

Comment 4

[Johnny Stenback (:jst)]

Not enough information here to make a blocking call here. I.e. what's the user impact, how likely will people run into this, etc. blocking- for now, but I'll approve the patch for uplift.

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 699236 [details] [diff] [review]
Bug 827860 - STK Ril throws and get stuck when default string is empty

[Triage Comment]

Comment 6

[Alexandre Poirot [:ochameau]]

Apart from the risk of being rejected during certification,
the STK menu breaks when you are trying to display an input without any default value. So that the STK menu is stuck in the input field screen, back won't work.
You can only escape from this by killing setting app, but if you try to relaunch the STK, nothing will work, tapping on any item will silently fail.

Comment 7

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

Comment on attachment 699236 [details] [diff] [review]
Bug 827860 - STK Ril throws and get stuck when default string is empty

Review of attachment 699236 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +8244,2 @@
>        input.defaultText = ctlv.value.textString;
>      }

My second thought would be,
since now we got the TLV with defaultText, 
we should also have defaultText property in input, 
but its value is still null.

Alex and Vicamo, how do you think?

Comment 8

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #7)
> Comment on attachment 699236 [details] [diff] [review]
> Bug 827860 - STK Ril throws and get stuck when default string is empty
> 
> Review of attachment 699236 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +8244,2 @@
> >        input.defaultText = ctlv.value.textString;
> >      }
> 
> My second thought would be,
> since now we got the TLV with defaultText, 
> we should also have defaultText property in input, 
> but its value is still null.
> 
> Alex and Vicamo, how do you think?

I found testString has the same issue when I wrote xpcshell tests.
So I think it makes sense to keep property and the value is null. (Both for defaultText and textString)
I make a new patch for this. will upload later.
Thanks
I make a new patch for this.

Comment 9

[Edgar Chen [:edgar]]

Attachment: Part 2:xpcshell tests for STK_CMD_GET_INPUT, v1

xpcshell tests

Comment 10

[Edgar Chen [:edgar]]

Attachment: Part 1: STK get stuck when text string is null, v2

address comment #8

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 699646 [details] [diff] [review]
Part 2:xpcshell tests for STK_CMD_GET_INPUT, v1

Review of attachment 699646 [details] [diff] [review]:
-----------------------------------------------------------------

approval-mozilla-b2g18 approved in comment #5

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/409dd3463c00
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e9f50b5c990

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/409dd3463c00
https://hg.mozilla.org/mozilla-central/rev/8e9f50b5c990

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/6e63a760fb95
https://hg.mozilla.org/releases/mozilla-b2g18/rev/7fda5fc80110