836257 - Be aware of GECKO_CARDSTATE_UNKNOWN new card state in Gaia apps.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[José Antonio Olivera Ortega [:jaoo]]

This is a follow-up bug of bug 822522. Since a new card state has been added to the possible ones we need to be sure that all apps that listen for `cardstatechange` event ( or where we switch/case the values of `mozMobileConnection.cardState` property) handle the SIM card states properly after landing bug 822522.

Comment 1

[Julien Wajsberg [:julienw]]

bug 822522 is tracking+ so nominating tracking-b2g18.

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Once this is done we should figure out something about bug 827741 also.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

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

Comment 4

[Jose Manuel Cantera]

:jaoo you have not requested a review. I can review the Contacts settings part and I think FCampo the FTU part.

Comment 5

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Jose M. Cantera from comment #4)
> :jaoo you have not requested a review. I can review the Contacts settings
> part and I think FCampo the FTU part.

Yep, I still need to figure out how the dialer app gets affected for the new SIM card states specially why it's not dealing with all different card states here [1]. I'll request review at some of you guys once I get that done. Thx!

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L42-L47

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

The logic for notifying ICC card status changes to content (bug 837755) has been updated a bit and a new state (`unknown`: transition between states) has been added. This PR is about reviewing (follow-up) the bits in Gaia where we check the ICC card status. Until we get r+ here bug 837755 should not land.

Comment 7

[Etienne Segonzac (:etienne)]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

r=me for the dialer changes

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

I feel like others review will be enough but I would like to understand the logic changes.

Comment 9

[Jose Manuel Cantera]

I checked the Contacts part and the FTU part and it was working ok. However I made a comment about a weird block of code in the settings part.

Comment 10

[Fernando Campo (:fcampo)]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

ok for the FTU part. thanks Jaoo :)

Comment 11

[Jose Manuel Cantera]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

with the nits typo addressed r+.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

https://github.com/mozilla-b2g/gaia/commit/c76e97d635f9a6205a9a3933a79e90e27b25f4c9

Comment 13

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 715464 [details]
Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/8186

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: ICC card state is being reported as 'absent' when airplane mode is on.
Testing completed: Yes.
Risk to taking this patch (and alternatives if risky): Low IMHO.
String or UUID changes made by this patch: Yes.

Comment 14

[Marcelino Veiga Tuimil [:sonmarce]]

This is the remaining work to fix bug 830287, that is already merged in v1-train. Well, there is no dependency between both of them, as no extra work is need in bug 830287, but this bug should be fixed to have the problem described in bug 830287 totally fixed. Sorry, maybe it sounds like a tongue twister, but I could not find a better explanation. :-)

Comment 15

[Marcelino Veiga Tuimil [:sonmarce]]

To uplift this bug to v1-train we also need bug 837755 to be a+, as it is the gecko part of the bug fixing and marked as a dependency.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

Please, don't uplift this until bugs 848765 and 837755 land on b2g18.

Comment 17

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #16)
> Please, don't uplift this until bugs 848765 and 837755 land on b2g18.

Sorry I meant bugs 822522 and 837755.

Comment 18

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

822522 is still not landed on b2g18, waiting for it land there.

Comment 19

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

Uplifted commit c76e97d635f9a6205a9a3933a79e90e27b25f4c9 as:
v1-train: 2a5ddb6758272227c1877b45984e053989189e36

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

(In reply to John Ford [:jhford] from comment #19)
> Uplifted commit c76e97d635f9a6205a9a3933a79e90e27b25f4c9 as:
> v1-train: 2a5ddb6758272227c1877b45984e053989189e36

This patch has been uplifted without doing the same with its gecko counterpart. I'll request checkin-needed for those bugs.

Comment 21

[José Antonio Olivera Ortega [:jaoo]]

Asking for tef+ flag. See bug 822522 comment #30

Comment 22

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

Uplifting 2a5ddb6758272227c1877b45984e053989189e36 results in a merge conflict.  It looks like it's mainly a conflict in cleanup that didn't land to 1.0.1

Comment 23

[José Antonio Olivera Ortega [:jaoo]]

(In reply to John Ford [:jhford] -- If you expect a reply from me, use needsinfo? instead of CC from comment #22)
> Uplifting 2a5ddb6758272227c1877b45984e053989189e36 results in a merge
> conflict.  It looks like it's mainly a conflict in cleanup that didn't land
> to 1.0.1

Thanks for trying the uplifting John, do you need a patch that applies cleanly in 1.0.1 gaia branch?

Comment 24

[Julien Wajsberg [:julienw]]

jaoo, you can simply try to uplift and push yourself, but if you're not comfortable, just push your commit to a branch in your repository and John or I would apply it.

Comment 25

[José Antonio Olivera Ortega [:jaoo]]

v1.0.1 https://github.com/mozilla-b2g/gaia/commit/cc0e498eca683258a926768388b5f39dd1c642a2

Comment 26

[Cody Roesch [:croesch]]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 27

[Deepa Subramanian]

Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?

Comment 28

[Julien Wajsberg [:julienw]]

Check that "unknown state" appears in the lockscreen when the phone just started.