Closed Bug 1154439 Opened 5 years ago Closed 4 years ago

[STK] TypeError: options.confirmMessage is undefined at: app://system.gaiamobile.org/gaia_build_defer_index.js

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S12 (15may)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: anshulj, Assigned: mancas)

References

Details

(Whiteboard: [caf priority: p2][CR 822883][FT:COMMS])

Attachments

(2 files)

STR
1. Simulate a STK Proactive command SETUP_CALL with no confirm message.

Expected: A default setup call message is displayed
Observed: TypeError: options.confirmMessage is undefined at: app://system.gaiamobile.org/gaia_build_defer_index.js

This is a regression from bug 1140153.
Manuel, the fix for this issue is to simply do a null check before calling options.confirmMessage.icons for both setup call and launch browser.

-      icc.asyncConfirm(message, confirmMessage, options.confirmMessage.icons,
+      icc.asyncConfirm(message, confirmMessage, (options.confirmMessage ? options.confirmMessage.icons : null),
         function(confirmed) {
           stkSetupCall(confirmed, callMessage);
         });
Flags: needinfo?(b.mcb)
Hi Wesley, need your help to triage this, thanks.
Flags: needinfo?(whuang)
Whiteboard: [FT:COMMS]
I suppose this is must-fix for CAF CS.
Keeping my NI to track.
blocking-b2g: 2.2? → 2.2+
Whiteboard: [FT:COMMS] → [CR 822883][FT:COMMS]
Whiteboard: [CR 822883][FT:COMMS] → [caf priority: p2][CR 822883][FT:COMMS]
Hi! Sean,

Please also help to take a look. Thanks

--
Keven
Flags: needinfo?(selee)
Assignee: nobody → b.mcb
Flags: needinfo?(b.mcb)
Comment on attachment 8593203 [details] [review]
[gaia] mancas:bug1154439 > mozilla-b2g:master

Hey Tim, could you review this patch when you get a chance?

Thanks =)
Attachment #8593203 - Flags: review?(timdream)
Attached patch unit_test.patchSplinter Review
Hi all,

I got a NI here, so here is my patch that add 2 unit tests to verify the case. Just for your reference. Thanks.
Flags: needinfo?(selee)
Status: NEW → ASSIGNED
Comment on attachment 8593203 [details] [review]
[gaia] mancas:bug1154439 > mozilla-b2g:master

Not covered by unit test?
Attachment #8593203 - Flags: review?(timdream) → feedback+
Comment on attachment 8593213 [details] [diff] [review]
unit_test.patch

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

::: apps/system/js/icc_worker.js
@@ +73,5 @@
>        if (STKHelper.isIconSelfExplanatory(options.confirmMessage)) {
>          confirmMessage = '';
>        }
> +      icc.asyncConfirm(message, confirmMessage,
> +        (options.confirmMessage ? options.confirmMessage.icons : null),

The same fix needs to be made for launch browser command.
Comment on attachment 8593203 [details] [review]
[gaia] mancas:bug1154439 > mozilla-b2g:master

Hey Sean, thanks for working on the unit test. I've added your code to my patch, so, Tim, you can review it when you want.

Thanks! =)
Attachment #8593203 - Flags: review?(timdream)
Attachment #8593203 - Flags: review?(timdream) → review+
Flags: needinfo?(whuang)
:mancas, can you please fill the approval request comment here?

Thanks

--
Keven
Flags: needinfo?(b.mcb)
Maria, can you coordinate with Manuel to request approval for landing?
Flags: needinfo?(oteo)
Flags: needinfo?(b.mcb)
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Comment on attachment 8593203 [details] [review]
[gaia] mancas:bug1154439 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):bug 1140153
[User impact] if declined: Confirm message of several STK messages won't be shown
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None
Attachment #8593203 - Flags: approval-gaia-v2.2?
(In reply to Dylan Oliver [:doliver] from comment #12)
> Maria, can you coordinate with Manuel to request approval for landing?

Manuel was waiting to request it after landing the patch in master.
Anyway, he has just asked the approval to 2.2, so clearing my ni. Cheers!
Flags: needinfo?(oteo)
Attachment #8593203 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Test case failed. Please fix at least the jshint error below. Thanks!

apps/system/test/unit/icc_worker_test.js: line 129, col 27, Strings must use singlequote. (ERROR)
apps/system/test/unit/icc_worker_test.js: line 131, col 34, Strings must use singlequote. (ERROR)
apps/system/test/unit/icc_worker_test.js: line 144, col 27, Strings must use singlequote. (ERROR)

There are many Gip errors but it seems not your fault...
Flags: needinfo?(b.mcb)
Flags: needinfo?(b.mcb)
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Manuel, I will manually merge your patch once you fix errors in tests (my comment 16).
Flags: needinfo?(b.mcb)
Hi Manuel, Thanks for fixing jshint error. The patch is merged:

landed on master: https://github.com/mozilla-b2g/gaia/commit/bb65538727c74f971c89c9d50aefa59c5b366bf0
gaia test: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=56a94d946029113cf80da6083b27caa20be54516
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
Flags: needinfo?(b.mcb)
You need to log in before you can comment on or make changes to this bug.