Closed Bug 962927 Opened 6 years ago Closed 6 years ago

[DSDS] After PUK is locked, SIM manager did not show "No SIM card" on the locked SIM and all outgoing settings are not changed to another SIM based on UX spec.

Categories

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

x86_64
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: echu, Assigned: eragonj)

References

Details

(Keywords: late-l10n, Whiteboard: dsdsrun1.3-2 dsdsrun1.4-1 [dolphin_test_wanted])

Attachments

(6 files)

Attached image 2014-01-23-10-39-38.png
Based on 1.3 DSDS v0.8 UX spec, page 44, item 3, once entering 10 incorrect PUK code, SIM cards manager shows "No SIM card" of the locked SIM and all the settings(outgoing voice, data and message) will be set to another on service SIM.

But on Fugu, it still shows "No operator" on locked SIM(SIM 1 in attached screenshot) and all settings are not changed to SIM2. 

Fugu
Gaia      ee25b0e45649d2955f340ce4f71ad55712dd0fab
Gecko     913cf2b92845441c9578787362ddad6f2ac15df6
BuildID   20140121095108
Version   28.0a2
Assignee: nobody → ejchen
I discussed with Edgar last time about this problem, and it seems that Fugu can't get `permanentBlocked` state from Gecko but would treat it as `unknown` state. 

Edgar, can you put comment down some investigations here about card state we talked few days ago ? Thanks !! :D
Flags: needinfo?(echen)
(In reply to EJ Chen [:eragonj][:小龍哥] from comment #3)
> I discussed with Edgar last time about this problem, and it seems that Fugu
> can't get `permanentBlocked` state from Gecko but would treat it as
> `unknown` state. 
> 
> Edgar, can you put comment down some investigations here about card state we
> talked few days ago ? Thanks !! :D

I had some trouble with my fugu environment last week. Still working on that, will update later once I fix it. Sorry for the late. :(
(In reply to EJ Chen [:eragonj][:小龍哥] from comment #3)
> I discussed with Edgar last time about this problem, and it seems that Fugu
> can't get `permanentBlocked` state from Gecko but would treat it as
> `unknown` state. 

In such case (entering 10th incorrect PUK code), fugu's rilc reports |app_state| as "6", there is no such define in AOSP standard [1]. So gecko treats it as 'unknown' state. Will file a new bug for this. Thank you. 
 
--
02-10 11:32:10.250   106   223 I Gecko   : RIL Worker[0]: iccStatus: {"cardState":1,"universalPINState":0,"gsmUmtsSubscriptionAppIndex":0,"cdmaSubscriptionAppIndex":8,"imsSubscriptionAppIndex":8,"apps":[{"app_type":2,"app_state":6,"perso_substate":0,"aid":null,"app_label":null,"pin1_replaced":0,"pin1":0,"pin2":0}]}
--

[1] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L471-L479
Flags: needinfo?(echen)
Hi Edgar and all:
 By following AOSP design, app_state will stay in RIL_APPSTATE_PUK, and pin1 state will be in RIL_PINSTATE_ENABLED_PERM_BLOCKE for PUK blocked case.


typedef enum {
    RIL_APPSTATE_UNKNOWN               = 0,
    RIL_APPSTATE_DETECTED              = 1,
    RIL_APPSTATE_PIN                   = 2, /* If PIN1 or UPin is required */
    RIL_APPSTATE_PUK                   = 3, /* If PUK1 or Puk for UPin is required */
    RIL_APPSTATE_SUBSCRIPTION_PERSO    = 4, /* perso_substate should be look at
                                               when app_state is assigned to this value */
    RIL_APPSTATE_READY                 = 5
} RIL_AppState;

typedef enum {
    RIL_PINSTATE_UNKNOWN = 0,
    RIL_PINSTATE_ENABLED_NOT_VERIFIED = 1,
    RIL_PINSTATE_ENABLED_VERIFIED = 2,
    RIL_PINSTATE_DISABLED = 3,
    RIL_PINSTATE_ENABLED_BLOCKED = 4,
    RIL_PINSTATE_ENABLED_PERM_BLOCKED = 5
} RIL_PinState;
See Bug 930394 as well.
Depends on: 970174
Comment on attachment 8371198 [details] [review]
patch on master

Hi Arthur, 

I just updated this flow in SimcardManager, and by the way, Fugu has also been fixed with incorrect card state issue. 

If you have time please help me review this part first and I will ask linear for some outdated simcards to test the scenario later !

Thanks :D
Attachment #8371198 - Flags: review?(arthur.chen)
Comment on attachment 8371198 [details] [review]
patch on master

Looks good to me. It seems that we don't handle "permanentBlocked" state before. There are several card state mapping tables in settings app. Could you help add this state to them? (you can grep "networkLocked" to find them)

Please request a review again once the card state change event is available. Thanks!
Attachment #8371198 - Flags: review?(arthur.chen)
(In reply to Arthur Chen [:arthurcc] from comment #9)
> Comment on attachment 8371198 [details] [review]
> patch on master
> 
> Looks good to me. It seems that we don't handle "permanentBlocked" state
> before. There are several card state mapping tables in settings app. Could
> you help add this state to them? (you can grep "networkLocked" to find them)
> 
> Please request a review again once the card state change event is available.
> Thanks!

Thanks Arthur, let me start to grep them out ! I would r? you again after its dependent bug 970174 got fixed.
Whiteboard: dsdsrun1.3-2 → dsdsrun1.3-2 dsdsrun1.4-1
blocking-b2g: --- → 1.4?
Wilfred,

Triage needs your input to understand on what the standard for DSDS on failover is?
Flags: needinfo?(wmathanaraj)
the expected behavior should be as described by the UX spec in comment 0
ni? UX Carrie to confirm
Flags: needinfo?(cawang)
Yes, please follow the spec. Thanks!
Flags: needinfo?(cawang)
triage: 1.4+ to fix SIM status and correct the behavior
blocking-b2g: 1.4? → 1.4+
(In reply to EJ Chen [:eragonj][:小龍哥] from comment #10)
> Thanks Arthur, let me start to grep them out ! I would r? you again after
> its dependent bug 970174 got fixed.

This bug is now red hot because it has no activity for 5 days. EJ, can we land this patch before PVOB issue is fixed? If not, I will try to push the vendor/TAM/EPM harder on that...
Flags: needinfo?(ejchen)
Hi Tim,

it seems that there are some problems between modem & gecko (Plz check bug 970174), based on my personal opinion, I prefer to land this bug after bug 970174 is fixed and it makes sense because without bug 970174, QA still can't verify this patch as well. 

Thanks :)
Flags: needinfo?(ejchen)
Commenting on Wilfred's behalf while he is out:
Blocking is the right decision here.
Flags: needinfo?(wmathanaraj)
Eric,

Based on comment 16, The work has been done in this bug and this bug won't be updated until the POVB bug 970174 is resolved. To avoid this bug being flagged in red in release dashboard, I suggest we can just land this bug first and put comment for QA to verify it after 970174 is landed. What do you think?
Flags: needinfo?(echang)
I have no problem to this.
Flags: needinfo?(echang)
Comment on attachment 8371198 [details] [review]
patch on master

Arthur I think this patch is ready to review ! I just checked the patch again, rebased to master and left some comments on PR. Please check it when you have time :) Thanks
Attachment #8371198 - Flags: review?(arthur.chen)
Comment on attachment 8371198 [details] [review]
patch on master

r=me, thank you!
Attachment #8371198 - Flags: review?(arthur.chen) → review+
Status: NEW → ASSIGNED
Thanks all, this patch got merged in Gaia/master : 5bb7daba0a3f0a3e694463001fd7063809bc2d17

I will keep this bug open and wait for its dependent bug got fixed so that QA can verify it.
Keywords: late-l10n
Looks like we are still waiting for the fix of bug 970174 which is marked with POVB.
Kevin, should we close this bug since it's landed?

Steven, do we need this patch/feature on 1.3T since it seems people are looking at Tarako on bug 970174?
Flags: needinfo?(styang)
Flags: needinfo?(khu)
This bug didn't land on 1.4, probably because it's not marked FIXED.

This is breaking string freeze, and we need to get moving on this.

PS: For 1.3, we're not expecting strings that are 1.3T only.
Attached file patch on v1.4
Arthur, 

this is the patch cherry-picked from master, please help me r+ this patch :)
Attachment #8403709 - Flags: review?(arthur.chen)
Comment on attachment 8403709 [details] [review]
patch on v1.4

r=me, thanks!
Attachment #8403709 - Flags: review?(arthur.chen) → review+
(In reply to Axel Hecht [:Pike] from comment #25)
> This is breaking string freeze, and we need to get moving on this.

I would put late-10n flag for v1.4 patch, thanks Pike.
Comment on attachment 8403709 [details] [review]
patch on v1.4

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 #): DSDS
[User impact] if declined: This is a edge case if users enter wrong simpin more than 10 times, the simcard will be blocked and simcardMnaager has to update its UI.
[Testing completed]: add needed unit tests already.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: There is one more string added in this patch - `simCardBlockedMsg` to hint users that this simcard is blocked.
Attachment #8403709 - Flags: approval-gaia-v1.4?(release-mgmt)
Comment on attachment 8403709 [details] [review]
patch on v1.4

Clarified some of the concerns I had on landing with :Ej offline. 

So we do need 970174( a POVB) issue to verify the bug, but since the gaia-patch is ready and is low-risk and because it introduces a late-l10n string, want to get it landed asap. we seem to have :pike's ok to get this landed from la0n side so approving this.
Attachment #8403709 - Flags: approval-gaia-v1.4?(release-mgmt) → approval-gaia-v1.4+
Thanks all, patches were all landed on 

gaia/master : 5bb7daba0a3f0a3e694463001fd7063809bc2d17
gaia/v1.4 : e1936c7a23fc075e033e8084173d0ceba2b39757

BTW, Eric will help me verify this bug after its dependent bug got fixed.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Sorry, I was in PTO. If it needs more information from me, please ni me again. Thanks.
Flags: needinfo?(khu)
Flags: needinfo?(styang)
Currently I am not able to verify this bug, Fugu is not supported anymore, Tarako is v1.3.
Whiteboard: dsdsrun1.3-2 dsdsrun1.4-1 → dsdsrun1.3-2 dsdsrun1.4-1 [dolphin_test_wanted]
Attached image DSC_1113.JPG
It shows "SIM Pin locked", and prompts for PUK when entering dialer. 

### v1.4 on <Dolphin>
Your Target Build: B2G.v1.4.0.dolphin     
Gaia      fccb15d6940db51615545574877a62d69406b1c2
Gecko     3ed3bbf1941e608bc9630942c7063a8b818f36bc
BuildID   20140505085052                  
Version   30.0
Hi, Carrie, Could you comment on this? Thanks.
Actual: "SIM Pin locked".
v1.4 Spec: not here.
v1.3 Spec: "No SIM card" from 962927#c0 above.
Flags: needinfo?(cawang)
This feature is postponed from 1.3 to 1.4 and it's implemented now (based on the previous comments). The behavior should be what I delivered in 1.3 spec. So please refer to it. thanks!
Flags: needinfo?(cawang)
Thanks, I found that doc.
(In reply to Carrie Wang [:carrie] from comment #36)
> This feature is postponed from 1.3 to 1.4 and it's implemented now (based on
> the previous comments). The behavior should be what I delivered in 1.3 spec.
> So please refer to it. thanks!
Hi, EJ, Could you check that please, thank you.
Flags: needinfo?(ejchen)
Flags: needinfo?(ejchen)
See Also: → 1006324
v1.4 Dolphin works fine. 
Gaia      f19735d288d9bf1c6ee0c0ecc7941421365037c7   
Gecko     67bcc125981ce4fda7b56187e08fc98113087daf   
BuildID   20140509085052                             
Version   30.0
Status: RESOLVED → VERIFIED
Thanks all and especially Eric for helping verifying the bug haha.

:P
You need to log in before you can comment on or make changes to this bug.