1005823 - [Dolphin] Cannot place emergency call w/o SIM card.

Status: RESOLVED FIXED

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

Description

[shawn ku [:sku]]

Attachment: screenshot.png

STR:
 1. Insert two valid SIMs
 2. Settings -> SIM manager -> Outgoing calls -> Always ask
 3. power down -> remove two SIMs -> power on
 4. place 112 via dialler

Symptom:
 See attached screenshot

Comment 1

[shawn ku [:sku]]

Hi Carrie:
 Could you please help comment which UX behaviour should we follow?

Thanks!!
Shawn

Comment 2

[Carrie Wang [New account: [e-mail]]]

Hi Shawn

Since it's for emergency call, it should be dialed out directly even there is no SIM inserted. Thanks!

Comment 3

[shawn ku [:sku]]

Hi Carrie:
 Yes, agree with you. however, Gaia did not know ECC or not until place call out. (if my memory is correct)

My original ni? is asking
Which SIM slot ID should we used for this case if gaia can identify ECC?

Pre-condition:
two SIMs inserted + always ask option

1. remove slot1 SIM, keep slot 2 case
2. remove slot2 SIM, keep slot 1 case
3. remove both slot1/slot2 SIM case


Hi Arthur:
 Please correct me if anything is wrong.

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

I think we can simply make emergency calls using the first sim slot without prompting anything to users.

Comment 5

[Ivan Tsay (:ITsay)]

Nominate this bug for v1.4 since the issue is critical and affect all DSDS phone including Flame.

Comment 6

[sam.hua]

it block our PTR1 test of Dolphin,please help to fix it asap.

Thanks!

Comment 7

[sam.hua]

it block our PTR1 test of Dolphin,please help to fix it asap.

Thanks!

Comment 8

[Carrie Wang [New account: [e-mail]]]

Hi, 
per discussion, the behaviour should be:

Pre-condition:
two SIMs inserted + always ask option

1. two SIM inserted:
Pop up the SIM picker for dialing emergency calls.

2. remove 1 SIM:
All the settings will be switched to the remain SIM. Hence, the emergency calls will be directly dialed via it (without ask).

3. remove both slot 0 and slot 1 case:
Slot 0 is the default SIM for emergency calls.

4. Airplane mode:
Slot 0 is the default SIM for emergency calls.

Thanks!

Comment 9

[Doug Sherk (:drs) (inactive)]

I believe this is a Gecko regression. I was able to repro this without calling an emergency number. I verified that we're passing |cardIndex| 0 here (which is as far as Gaia goes):
https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/telephony_helper.js#L97

Despite the blocking flag, I don't think this is actually a v1.4 Gaia bug. I've verified this bug with the following:

v1.4 Gaia, master Gecko
master Gaia, master Gecko

Hsin-Yi, can you take a look or point us to someone who can? Thanks.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Doug Sherk (:drs) from comment #9)
> I believe this is a Gecko regression. I was able to repro this without
> calling an emergency number. I verified that we're passing |cardIndex| 0
> here (which is as far as Gaia goes):
> https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/
> telephony_helper.js#L97
> 
> Despite the blocking flag, I don't think this is actually a v1.4 Gaia bug.
> I've verified this bug with the following:
> 
> v1.4 Gaia, master Gecko
> master Gaia, master Gecko
> 
> Hsin-Yi, can you take a look or point us to someone who can? Thanks.

Sure. I'll keep you posted.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Carrie Wang [:carrie] from comment #8)
> Hi, 
> per discussion, the behaviour should be:
> 
> Pre-condition:
> two SIMs inserted + always ask option
> 
> 1. two SIM inserted:
> Pop up the SIM picker for dialing emergency calls.
> 
> 2. remove 1 SIM:
> All the settings will be switched to the remain SIM. Hence, the emergency
> calls will be directly dialed via it (without ask).
> 
> 3. remove both slot 0 and slot 1 case:
> Slot 0 is the default SIM for emergency calls.
> 
> 4. Airplane mode:
> Slot 0 is the default SIM for emergency calls.
> 

4. isn't true. Slot 0 isn't necessarily the default SIM in airplane mode. We should still check if there's a card inserted. If yes, use the specified default SIM.


> Thanks!

Comment 12

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #11)
> (In reply to Carrie Wang [:carrie] from comment #8)
> > Hi, 
> > per discussion, the behaviour should be:
> > 
> > Pre-condition:
> > two SIMs inserted + always ask option
> > 
> > 1. two SIM inserted:
> > Pop up the SIM picker for dialing emergency calls.
> > 
> > 2. remove 1 SIM:
> > All the settings will be switched to the remain SIM. Hence, the emergency
> > calls will be directly dialed via it (without ask).
> > 
> > 3. remove both slot 0 and slot 1 case:
> > Slot 0 is the default SIM for emergency calls.
> > 
> > 4. Airplane mode:
> > Slot 0 is the default SIM for emergency calls.
> > 
> 
> 4. isn't true. Slot 0 isn't necessarily the default SIM in airplane mode. We
> should still check if there's a card inserted. If yes, use the specified
> default SIM.
> 

This is the device implementation issue. But it looks the code [1] does the right way :)

[1] https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/telephony_helper.js#L94

> 
> > Thanks!

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Doug Sherk (:drs) from comment #9)
> I believe this is a Gecko regression. I was able to repro this without
> calling an emergency number. I verified that we're passing |cardIndex| 0
> here (which is as far as Gaia goes):
> https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/
> telephony_helper.js#L97
> 
> Despite the blocking flag, I don't think this is actually a v1.4 Gaia bug.
> I've verified this bug with the following:
> 
> v1.4 Gaia, master Gecko
> master Gaia, master Gecko
> 
> Hsin-Yi, can you take a look or point us to someone who can? Thanks.

Hi Doug,

I read through the whole comment and the STR, and don't know why you believe this is a gecko regression.

Per STR in comment 0 and the attached screenshot.png, SIM picker is popped out even there's no sim card at all. And as there's no sim card, the selection menu is empty so that user has no way to make out a call. 

When I was reproducing the issue, [1] was not reached, and telephony.dial() API was never called, either. I tried to trace the gaia code but didn't get an idea why sim picker menu still shows up when there's no sim. Could you please check gaia part again?

Also, if we don't follow STR, i.e. not choosing "ALWAYS ASK", before taking out the cards from the device, then I could successfully make out a call/emergency call when no cards inserted. Gecko works well.

[1] https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/telephony_helper.js#L97

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Okay, I think I know where goes wrong.

[1] checks if it hits the case of one sim card. However, if there's no card, [2] will still be reached out. We need to consider the case of no card individually.

[1] https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/multi_sim_action_button.js#L63
[2] https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/multi_sim_action_button.js#L71

Comment 15

[Doug Sherk (:drs) (inactive)]

(In reply to Hsin-Yi Tsai (OOO 5/30 ~ 6/8) [:hsinyi] from comment #13)
> Hi Doug,
> 
> I read through the whole comment and the STR, and don't know why you believe
> this is a gecko regression.
> 
> Per STR in comment 0 and the attached screenshot.png, SIM picker is popped
> out even there's no sim card at all. And as there's no sim card, the
> selection menu is empty so that user has no way to make out a call. 
> 
> When I was reproducing the issue, [1] was not reached, and telephony.dial()
> API was never called, either. I tried to trace the gaia code but didn't get
> an idea why sim picker menu still shows up when there's no sim. Could you
> please check gaia part again?
> 
> Also, if we don't follow STR, i.e. not choosing "ALWAYS ASK", before taking
> out the cards from the device, then I could successfully make out a
> call/emergency call when no cards inserted. Gecko works well.
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/
> telephony_helper.js#L97

Thanks for looking into this. It's possible that the issue I was reproing was something different (since I used different STR), and I assumed that they had the same root cause. Your analysis makes sense for the emergency call case. I'm going to look more into the other issue I was reproing.

Comment 16

[Doug Sherk (:drs) (inactive)]

Attachment: Fix emergency calls with no SIM card.

(In reply to Hsin-Yi Tsai (OOO 5/30 ~ 6/8) [:hsinyi] from comment #14)
> Okay, I think I know where goes wrong.
> 
> [1] checks if it hits the case of one sim card. However, if there's no card,
> [2] will still be reached out. We need to consider the case of no card
> individually.
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/
> multi_sim_action_button.js#L63
> [2]
> https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/
> multi_sim_action_button.js#L71

Sorry for wasting your time Hsin-Yi, I completely messed up on this and in my head I was merging this issue with bug 1008974. There's actually nothing in common between them. :)

PR: https://github.com/mozilla-b2g/gaia/pull/19727

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Doug Sherk (:drs) from comment #16)
> Created attachment 8430064 [details] [diff] [review]
> Fix emergency calls with no SIM card.
> 
> (In reply to Hsin-Yi Tsai (OOO 5/30 ~ 6/8) [:hsinyi] from comment #14)
> > Okay, I think I know where goes wrong.
> > 
> > [1] checks if it hits the case of one sim card. However, if there's no card,
> > [2] will still be reached out. We need to consider the case of no card
> > individually.
> > 
> > [1]
> > https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/
> > multi_sim_action_button.js#L63
> > [2]
> > https://github.com/mozilla-b2g/gaia/blob/v1.4/shared/js/
> > multi_sim_action_button.js#L71
> 
> Sorry for wasting your time Hsin-Yi, 

No problem! Glad that we got the root cause eventually :)

> I completely messed up on this and in
> my head I was merging this issue with bug 1008974. There's actually nothing
> in common between them. :)
> 
> PR: https://github.com/mozilla-b2g/gaia/pull/19727

Comment 18

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8430064 [details] [diff] [review]
Fix emergency calls with no SIM card.

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

::: shared/js/multi_sim_action_button.js
@@ +60,5 @@
>      return;
>    }
>  
> +  if (navigator.mozIccManager.iccIds.length <= 1) {
> +    this.performAction(0);

Hi, sorry to jump in. 
This line isn't right. iccIds index isn't the same as 'cardIndex' we require for dialing a call. We should rely on the index of mobileConnections, where the corresponding element detects a card. Sorry that the API isn't clear enough :(

Comment 19

[Doug Sherk (:drs) (inactive)]

Attachment: Fix emergency calls with no SIM card.

(In reply to Hsin-Yi Tsai (OOO 5/30 ~ 6/8) [:hsinyi] from comment #18)
> Hi, sorry to jump in. 
> This line isn't right. iccIds index isn't the same as 'cardIndex' we require
> for dialing a call. We should rely on the index of mobileConnections, where
> the corresponding element detects a card. Sorry that the API isn't clear
> enough :(

Thanks! Does this fix look reasonable to you then? https://github.com/DouglasSherk/gaia/commit/7016117318ad5795f3bee4fa3a8e48db74698b0e#diff-f9110901d30ac29f4ff93925255d9a5eR63

Comment 20

[Anthony Ricaud (:rik)]

Comment on attachment 8433039 [details] [diff] [review]
Fix emergency calls with no SIM card.

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

Sorry for the slow review, this patch needs rebasing because of that. Please send me an email when you put it in review again so I can put it on top of my todo list.

Comment 21

[Doug Sherk (:drs) (inactive)]

Attachment: Fix emergency calls with no SIM card.

Rebased, updated PR.

Comment 22

[Hsin-Yi Tsai (she/her) [:hsinyi]]

The new PR looks reasonable to me, thanks!

Comment 23

[Anthony Ricaud (:rik)]

Comment on attachment 8436527 [details] [diff] [review]
Fix emergency calls with no SIM card.

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

This is a bit complicated for what we want here. The for loop you're doing to find out which SIM card is inserted is already handled by some code in the system app that sets the preferred card.

Therefore, I think the code should read like this:
var cardIndex = getCardIndex()
if (navigator.mozIccManager.iccIds.length === 0) { performAction(0) } // we don't care, the call will go through if it's an emergency number (that fixes this bug)
if (cardIndex == ALWAYS_ASK) { displaySimPicker() }
else { performAction(cardIndex) } // this fixes bug 1019488

::: apps/communications/dialer/test/unit/multi_sim_action_button_test.js
@@ +158,5 @@
>  
> +    test('should use first SIM slot (which is empty)', shouldUsePrimarySimCard);
> +  });
> +
> +  suite('1 SIM', function() {

To test both slot 0 and 1, can you wrap this in a [0, 1].forEach() ? Like at https://github.com/mozilla-b2g/gaia/blob/86da626923719d6550b4e0fccc67af27f96a02e4/apps/communications/dialer/test/unit/call_log_test.js#L807-L831

Comment 24

[Doug Sherk (:drs) (inactive)]

Attachment: Fix emergency calls with no SIM card.

Addressed review comments, updated PR. You're right that it was overly complicated. I'm not sure why we ever special cased the length === 1 situation to begin with.

Comment 25

[Anthony Ricaud (:rik)]

Comment on attachment 8437152 [details] [diff] [review]
Fix emergency calls with no SIM card.

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

If I disable the code in the if (0) branch, the tests are not failing.

Also, you can probably remove all the mozMobileConnections setup in the tests since we're not using this API anymore.

Comment 26

[Doug Sherk (:drs) (inactive)]

Attachment: Fix emergency calls with no SIM card.

Updated PR. The 0 SIM test in this now fails if you disable the SIMs.length === 0 check in MSAB.

Comment 27

[Anthony Ricaud (:rik)]

Comment on attachment 8437979 [details]
Fix emergency calls with no SIM card.

Just a tiny title change for readability.

Comment 28

[Doug Sherk (:drs) (inactive)]

master: https://github.com/mozilla-b2g/gaia/commit/61b732ecd5b4682f22ab6aeb5311c2f6e026a084
v2.0: https://github.com/mozilla-b2g/gaia/commit/3503185c6829ac89d114b6c652691bfaac1163f8
v1.4: https://github.com/mozilla-b2g/gaia/commit/9379ddab393eb09cb175cd9418f58b76b9951842

Comment 29

[Coler]

This issue has been verified successfully on Flame2.0&2.1.
Reproducing rate: 0/5
See attachment: Verify_Flame_ECC.mp4

Flame2.0 build version:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141201000201
Version         32.0

Flame2.1 build version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141201001201
Version         34.0

Comment 30

[Coler]

Attachment: Verify_Flame_ECC.MP4