923161 - Emergency calls not performed with some "no-line" SIM cards

Status: RESOLVED FIXED

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

Description

[Juan Perez-Bedmar [:juanpbf]]

Found in ikura
QC RIL version:
"ro.build.firmware_revision=01.01.00.019.215"
gaia commit:
4584c40 rm long short
gecko commit:
8ae9e11 tengfei add eng code with 87274


When using a SIM card with no line, you cannot dial emergency numbers (112/911) This was tested happens with different SIM cards which does not camp in real network:
- Anite SAS (lab) SIM card
- Vodafone SIM card (no line)
- Vivo SIM card (no line)
- Telenor SIM card (no line)


STEPS TO REPRODUCE
1) Insert a SIM card whose line has been cancelled (If you attended last WW in Oslo or the previous one in Madrid, and you keep the SIM card we were given there, you can use it for this test)
2) Try to dial 112 or 911


EXPECTED BEHAVIOUR
The call should be established


CURRENT BEHAVIOUR
Nothing happens, the dialler screen remains unaltered and the number is not dialled


ADDITIONAL INFO
- This seems to be a regression as it doesn't happened in V1.0.1
- This has been reported as a cert. blocker by the operator

--> as per above comments, this bug should be nominated to leo

Comment 1

[Beatriz Rodríguez [:brg]]

This is a certification blocker I can reproduce using:
unagi, MOZ RIL, v1-train: Gecko-44e9faa.Gaia-00757c6.
unagi, MOZ RIL, v1.2:Gecko-f0e33a4.Gaia-7447624 

This is a regression, it worked fine in v1.0.1.

Comment 2

[Beatriz Rodríguez [:brg]]

Sorry I made a mistake when copying the commits, this is the suitable information. Sorry for any inconvenience:
unagi, MOZ RIL, v1-train: Gecko-44e9faa.Gaia-00757c6
unagi, MOZ RIL, v1.2: Gecko-a55de78.Gaia-9e21b6b

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v1

This issues is because the dialer app doesn't allow to perform emergency calls when the ICC card state is unknown. If emergency calls are not allowed that would be right but the dialer doesn't check whether emergency calls are allowed at all. This patch adds the bits for performing the check and allows the dialer to perform the call even when the ICC card status is unknown.

FYI, this issue is not happening in master (both gecko and gaia) with the same ICC cards used to test this certification requirement. It's not happening
because we landed bug 916000 and the ICC card status reported for these ICC cards is now 'illegal'.

Comment 4

[Beatriz Rodríguez [:brg]]

We need this patch landing in v1-train and v1.2. Setting the suitable flags.

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 813710 [details] [diff] [review]
v1

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

The change looks good but we need to add a test for this.

It should be pretty close to the dialEmergency test [1].
But you'll need to stub IccHelper.cardState to return unknown or illegal.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/test/unit/telephony_helper_test.js#L69-76

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v2

Address review comments (comment #5).

Sutbbing getter/setter methods in Sinon is not straightforward so I've ended up touching the MockIccHelper object a bit. I hope you are ok with this approach.

Comment 7

[Etienne Segonzac (:etienne)]

Comment on attachment 814332 [details] [diff] [review]
v2

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

Yes, you did the right thing with the MockIccHelper.

r=me with the better mCardState restore.

Thanks!

::: apps/communications/dialer/test/unit/telephony_helper_test.js
@@ +66,5 @@
>      mockTelephony.verify();
>    });
>  
> +  suite('should dialEmergency if the card state is unknown',
> +    function() {

you could have a
`var initialState;` here

@@ +67,5 @@
>    });
>  
> +  suite('should dialEmergency if the card state is unknown',
> +    function() {
> +      setup(function() {

`initialState = MockIccHelper.mCardState;` here

@@ +72,5 @@
> +        MockIccHelper.mCardState = 'unknown';
> +      });
> +
> +      teardown(function() {
> +        MockIccHelper.mCardState = 'ready';

`MockIccHelper.mCardState = initialState;` here

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v3 r=etienne

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/12741

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

https://github.com/mozilla-b2g/gaia/commit/0fff8ad020a1934cae557155c951d73fb0887b8f

Comment 11

[Beatriz Rodríguez [:brg]]

Jaoo, thanks for your fast response to fix this bug.
John, please make the uplift to v1-train and v1.2. Thanks in adavance.

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

[v1.2 585eb22]

There were conflicts on V1-Train.

#	deleted by us:      apps/communications/dialer/test/unit/mock_icc_helper.js
#	both modified:      apps/communications/dialer/test/unit/telephony_helper_test.js

Comment 13

[Beatriz Rodríguez [:brg]]

José Antonio, please take a look to the conflict. Thanks in advance.

Comment 14

[José Antonio Olivera Ortega [:jaoo]]

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #12)
> [v1.2 585eb22]
> 
> There were conflicts on V1-Train.
> 
> #	deleted by us:      apps/communications/dialer/test/unit/mock_icc_helper.js
> #	both modified:     
> apps/communications/dialer/test/unit/telephony_helper_test.js

Etienne, It seems the test cannot be uplifted so would you be ok with uplifting this patch to v1-train branch without the test? Thanks.

Comment 15

[Etienne Segonzac (:etienne)]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #14)
> (In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC
> from comment #12)
> > [v1.2 585eb22]
> > 
> > There were conflicts on V1-Train.
> > 
> > #	deleted by us:      apps/communications/dialer/test/unit/mock_icc_helper.js
> > #	both modified:     
> > apps/communications/dialer/test/unit/telephony_helper_test.js
> 
> Etienne, It seems the test cannot be uplifted so would you be ok with
> uplifting this patch to v1-train branch without the test? Thanks.

Yes, if the uplifting issue is v1-train only and we do a quick manual test we can land the simple fix itself without the test.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

v1-train: https://github.com/mozilla-b2g/gaia/commit/db1cb176b94f53a59db78490bd379e96fc0c66f2

We should uplift this bug to v1.1hd branch as well, shouldn't we?

Comment 17

[Beatriz Rodríguez [:brg]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #16)
> v1-train:
> https://github.com/mozilla-b2g/gaia/commit/
> db1cb176b94f53a59db78490bd379e96fc0c66f2
> 
> We should uplift this bug to v1.1hd branch as well, shouldn't we?
Yes, please uplift to v1.1hd as well.

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: db1cb176b94f53a59db78490bd379e96fc0c66f2
v1.1.0hd: 5af5111d04c0b2f30ca0fc69daa28416537e3702