Closed Bug 827860 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21
blocking-basecamp -
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed

People

(Reporter: ochameau, Assigned: edgar)

References

Details

Attachments

(2 files, 1 obsolete file)

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}
Attachment #699236 - Flags: review?(vyang)
Nomitating as STK is heavily tested during certification and we may hit that exception.
blocking-basecamp: --- → ?
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. :)
Attachment #699236 - Flags: review?(vyang) → review+
Assignee: nobody → poirot.alex
OS: All → Gonk (Firefox OS)
Hardware: All → ARM
Summary: STK Ril throws and get stuck when default string is empty → B2G STK: Ril throws and get stuck when default string is empty
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.
blocking-basecamp: ? → -
Comment on attachment 699236 [details] [diff] [review]
Bug 827860 - STK Ril throws and get stuck when default string is empty

[Triage Comment]
Attachment #699236 - Flags: approval-mozilla-b2g18+
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 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?
Summary: B2G STK: Ril throws and get stuck when default string is empty → B2G STK: Ril throws and get stuck when default text is null
(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.
xpcshell tests
address comment #8
Attachment #699236 - Attachment is obsolete: true
Assignee: poirot.alex → echen
Attachment #699646 - Attachment description: xpcshell tests for STK_CMD_GET_INPUT, v1 → Part 2:xpcshell tests for STK_CMD_GET_INPUT, v1
Attachment #699648 - Attachment description: STK get stuck when text string is null, v2 → Part 1: STK get stuck when text string is null, v2
Attachment #699646 - Flags: review?(vyang)
Attachment #699648 - Flags: review?(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
Attachment #699646 - Flags: review?(vyang)
Attachment #699646 - Flags: review+
Attachment #699646 - Flags: approval-mozilla-b2g18?
Attachment #699648 - Flags: review?(vyang)
Attachment #699648 - Flags: review+
Attachment #699648 - Flags: approval-mozilla-b2g18?
Attachment #699646 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Attachment #699648 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/mozilla-central/rev/409dd3463c00
https://hg.mozilla.org/mozilla-central/rev/8e9f50b5c990
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.