Closed
Bug 857837
Opened 12 years ago
Closed 11 years ago
Enter NCK/CCK/SPCK popup should pop up at boot up if phone is in network/corporate/serviceprovider locked state
Categories
(Firefox OS Graveyard :: Gaia::System, defect, P3)
Tracking
(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.1 fixed)
People
(Reporter: nsarkar, Assigned: julienw)
References
Details
(Whiteboard: QARegressExclude)
Attachments
(3 files, 1 obsolete file)
The NCK/CCK/SPCK pop up for entering the code should show up at bootup. Currently, the pop up shows up after sometime (20 sec) and during that time, user can access other apps on the phone. This is not correct behavior because when a phone is locked to a network or service provider, it should not be useable with a different SIM (mcc+ mnc).
Bug 832814 is the cause of this problem.
Nivi, whether or not the user is allowed to use the phone for non telephony related functionality is OS dependent. In case of FFOS the user can use the phone for example if the SIM is PIN locked however Android doesn't let user go past the lock screen.
The real issue here is that NCK/CCK/SPCK popup doesn't happen at the boot time but after 20secs which seems pretty weird.
Assignee | ||
Comment 3•12 years ago
|
||
The problem in Bug 832814 was that the NCK popup was happening even if the user gave a good PIN code. The underlying problem was that the RIL was reporting a "networkLocked" status and we were waiting for 5 seconds which was not enough.
If it's possible to have a "registering" status from the RIL while it's trying to register with the pin, then I suppose we can get rid of this timeout at all.
Also, we can be smarter and have a timeout only if we got pinRequired before. But I think this logic should actually be in the RIL or at least the API instead...
We can also lower it at 10s because as I said in Bug 832814 comment 8 that value worked for me too but since it is a time race condition I wanted to feel safer.
Flags: needinfo?(josea.olivera)
Comment 4•12 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #3)
> The problem in Bug 832814 was that the NCK popup was happening even if the
> user gave a good PIN code. The underlying problem was that the RIL was
> reporting a "networkLocked" status and we were waiting for 5 seconds which
> was not enough.
We never had a SIM-locked phone to test this properly and there was the issue about the RIL reporting networkLocked state while the SIM PIN dance.
> If it's possible to have a "registering" status from the RIL while it's
> trying to register with the pin, then I suppose we can get rid of this
> timeout at all.
>
> Also, we can be smarter and have a timeout only if we got pinRequired
> before. But I think this logic should actually be in the RIL or at least the
> API instead...
The RIL plumbing underneath is receiving `networkLocked` SIM card status in devices that don't have such SIM-locked feature on. Do you Anshul why this is possible? IHMO that's not makes sense and this is truly issue here.
> We can also lower it at 10s because as I said in Bug 832814 comment 8 that
> value worked for me too but since it is a time race condition I wanted to
> feel safer.
SGTM
Flags: needinfo?(josea.olivera)
Jose/Julien, RIL is reporting the state of networkLocked as a transient state with substate of ready, which is just a transient state stating that nothing is pending for perso and is part of SIM initialization sequence.
Commercial RIL handles this transient state information correctly and so I would request you to please back out the bug 832814 for the reasons states by nsarakar in this bug.
Network locked feature is supported on v1 and so requesting for TEF+ as this issue causes the NCK bug to pop up after 20 secs of bootup instead of right away.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Anshul from comment #5)
> Jose/Julien, RIL is reporting the state of networkLocked as a transient
> state with substate of ready, which is just a transient state stating that
> nothing is pending for perso and is part of SIM initialization sequence.
>
> Commercial RIL handles this transient state information correctly and so I
> would request you to please back out the bug 832814 for the reasons states
> by nsarakar in this bug.
sorry but what does "correctly" mean exactly? could you please elaborate on which states the ril reports when entering a PIN code? IMHO the pin code use case is a lot more important than the nck use case.
Julien, when app state is perso locked but perso substate is ready, this should be not be considered as network locked. In case where phone is genuinely locked to the network the app state reported would be perso locked but perso substate reported would not be ready but would be say network locked for NCK.
For pin locked, the app state will always be RIL_APPSTATE_PIN and is not effected by bug 832814, which is only for NCK.
Assignee | ||
Comment 10•12 years ago
|
||
Anshul, if I understand correctly, in Bug 832814, we should have checked the "perso substate" instead of raising the timeout ? Do we have this information in the API ? If yes I also think we should backout the patch in Bug 832814 and reopen that bug to fix it the correct way.
Comment 11•12 years ago
|
||
Anshul, do you mean that we are not reporting `networkLocked` ICC card state properly? I mean do we have to check perso substate as well?
Sadly current mozMobileConnection API doesn't provide such information (perso substate) so this is a matter of checking perso substate before reporting a truly `networkLocked` ICC card state. If I understand what Anshul is telling us. If so I guess we will be able to get rid of all these timeout workarounds at all.
Comment 12•12 years ago
|
||
Jose, yes you need to check for perso substate correctly before reporting network locked.
Julie, this should be taken care of by gecko and not gaia so you need not check for perso substate in gaia to display the network locked UI or not.
Comment 13•12 years ago
|
||
Hi,
This is not a blocker for certification, but a nice-to-have.
Thks!!
David
Flags: needinfo?(dpv)
Comment 14•12 years ago
|
||
tef- unless our partners disagree
blocking-b2g: tef? → ---
Flags: needinfo?(mvines)
Comment 15•12 years ago
|
||
It's pretty bad behaviour and the fix for this bug is simple/low risk for v1.0.1 commercial devices, back out the silly 20 second timer introduced by bug 710962, but I'm not sure if this issue would be blocking for all the target markets.
Flags: needinfo?(mvines)
Assignee | ||
Comment 16•12 years ago
|
||
I think you wanted to mention Bug 832814.
simply backing out that bug is not good as I believe the bug it fixes is more important than this one, and I don't want to bring it back.
before removing the timeout we need a gecko patch. Antonio, do you think you could do that?
Comment 17•12 years ago
|
||
Sorry, yeah wrong bug :)
Yes it seems like the reference RIL does need that 20 second hack from bug 832814, but the commercial devices will be fine without it.
Assignee | ||
Comment 18•12 years ago
|
||
ok then I suggest that we remove the timeout in 1.0.1 only, until we fix Mozilla's RIL. does that sound good?
Comment 19•12 years ago
|
||
I suggest doing the same on 1.1 as well.
Comment 20•12 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #16)
> I think you wanted to mention Bug 832814.
>
>
> simply backing out that bug is not good as I believe the bug it fixes is
> more important than this one, and I don't want to bring it back.
>
>
> before removing the timeout we need a gecko patch. Antonio, do you think you
> could do that?
Sure, but I have other blockers. I'll work on this asap. IMHO we must fix the platform issue before dealing with other workarounds.
Assignee | ||
Comment 21•12 years ago
|
||
Michael, now that we have a clear path to fixing this bug, can we wait to fix the underlying problem on the Mozilla RIL before removing/lowering the setTimeout ?
Comment 22•12 years ago
|
||
(In reply to Anshul from comment #12)
> Jose, yes you need to check for perso substate correctly before reporting
> network locked.
This is already done. It was implemented by Yoshi and the code lives in mozilla-cental. See bug 849757 please. I guess that we could have this issue solved by uplifting bug 849757 to the corresponding release branches. I still have to test it but does it seem a solution?
Flags: needinfo?(anshulj)
See Also: → 849757
Comment 23•12 years ago
|
||
If uplifting bug 849857 is what needs to happen to back out bug 832814 than we should do it. Bug 849857 carries no risk to the product as it only touches the reference RIL implementation, and RIL IDL file changes have no affect on binary compatibility between Gecko and the commercial RIL.
Comment 24•12 years ago
|
||
Also bug 832814 can already be backed out on v1.1 as bug 849857 is already available for v1.1.
Flags: needinfo?(anshulj)
Comment 25•12 years ago
|
||
s/849857/849757/ in ^^^^ of course. I'm getting pretty good at mistyping bug #s it seems :(
Comment 26•12 years ago
|
||
If we uplift bug 849757 definitely I'd prefer to remove the timeout because it wouldn't be needed. I'll do a test and keep you updated.
Comment 27•12 years ago
|
||
leo+, as the solution now seems trivial for v1.1 -- please back out bug 832814.
We may want to consider the same for v1.0.1 as this bug has been reported there as well, and support for this in the v1.0.1 commercial RIL is already present so the risk is minimal.
blocking-b2g: --- → leo+
Assignee | ||
Comment 28•12 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #26)
> If we uplift bug 849757 definitely I'd prefer to remove the timeout because
> it wouldn't be needed. I'll do a test and keep you updated.
I wonder if the timeout is necessary so that it doesn't try to display too early while the phone is booting.
For v1 I'd prefer to stay on the safe side and simply backout bug 832814. For master we can do more experiments.
It's a Gaia bug.
Assignee: allstars.chh → nobody
Assignee | ||
Comment 31•12 years ago
|
||
I'll do it as I'm the culprit in Bug 832814.
Assignee: nobody → felash
Bug 832814 is landed before Bug 849757.
However I think Bug 849757 did fix the root cause (For Mozilla RIL).
So my opinion is to backout Bug 832814 as well.
Also comment 8 from Ansull says
"when app state is perso locked but perso substate is ready, this should *not* be considered as network locked."
Bug 849757 Part 2 patches addresses this point.
(https://bugzilla.mozilla.org/attachment.cgi?id=724774&action=diff ril_worker.js line 2861)
Assignee | ||
Comment 33•12 years ago
|
||
If this bug ever needs tef+, then bug 849757 will need tef+ too.
Assignee | ||
Comment 34•12 years ago
|
||
After careful testing, I completely removed the setTimeout hack.
However I've no simlocked phone so I'd like that someone that do have a simlocked phone try this patch and tell me if it works in all situations. Especially I'd like to know if the NCK popup is displayed when the phone boots in this situation.
---
apps/system/js/sim_lock.js | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
(see also PR https://github.com/mozilla-b2g/gaia/pull/9118)
Attachment #736294 -
Flags: review?(josea.olivera)
Flags: needinfo?(anshulj)
Comment 35•12 years ago
|
||
Comment on attachment 736294 [details] [diff] [review]
patch v1
Review of attachment 736294 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for the patch Julien, that's the idea. I've applied and tested the patch and it seems everything is fine. The 'networkLocked' SIM card state is no longer reported during the SIM initialization dance (remember it was in the past). I'd would be nice to have a double-check from Anshul since he is able to test it on a SIM-locked device.
::: apps/system/js/sim_lock.js
@@ +96,1 @@
> break;
Delete the break statement and return true here please.
Attachment #736294 -
Flags: review?(josea.olivera) → review+
Comment 36•12 years ago
|
||
Nivi, please give this a test?
This bug also needs to be uplifted to tef as TEF partners have already raised this issue twice. Please see the "Blocks" section of this bug. Commercial RIL already has a equivalent fix for Bug 849757 on v1 so this should be a low risk for v1 too. Nsarkar can test this on both v1 and v1.1 and let you know.
Flags: needinfo?(anshulj) → needinfo?(nsarkar)
Assignee | ||
Comment 40•12 years ago
|
||
carrying r=jaoo
addressed the review comment.
Also updated the PR in https://github.com/mozilla-b2g/gaia/pull/9118
Attachment #736294 -
Attachment is obsolete: true
Attachment #736715 -
Flags: review+
Updated•12 years ago
|
Comment 41•12 years ago
|
||
I'd like to sum up guys how we should uplift this work and all its dependencies (both
gecko and gaia ones).
1.- Land bug 822522 patch in mozilla-b2g18_v1_0_1 release branch.
2.- Land bug 836257 patch in gaia v1.0.1 release branch.
3.- Land bug 849757 patches in mozilla-b2g18_v1_0_1 release branch.
4 .-Land this work (once Nivi checks if everything is fine) in gaia {master,
v1-train, v1.0.1} release branches.
Am I right?, please correct me if I am wrong. Thanks!
Assignee | ||
Comment 42•12 years ago
|
||
looks good to me.
Comment 43•12 years ago
|
||
Should we then also lift bug 837755 to v1.0.1?
Assignee | ||
Comment 44•12 years ago
|
||
I think this is not an obligation, I think it was not a regression produced by bug 822522.
(but we still can).
Reporter | ||
Comment 45•12 years ago
|
||
Hi guys,
I have verified the fix works on v1 and v1.1.
Thanks,
Nivi.
Flags: needinfo?(nsarkar)
Assignee | ||
Comment 46•12 years ago
|
||
master: 3fa788342953f33ab980fa1ba28deba1a1b8e650
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 47•12 years ago
|
||
v1-train: bc1fd377bf73fa8b6af19a39b20866ffc203b87b
Assignee | ||
Comment 48•12 years ago
|
||
waiting for the uplift of Bug 849757 before uplifting this to v1.0.1.
Assignee | ||
Comment 49•12 years ago
|
||
Bug 849757 was uplifted today. James, would you please uplift this ?
Flags: needinfo?(jlal)
Comment 50•12 years ago
|
||
:( Sorry this does not uplift cleanly still.
Flags: needinfo?(jlal) → needinfo?(felash)
Assignee | ||
Comment 51•12 years ago
|
||
v1.0.1: 2d86d82d38355c68cc4506b69d3b44042534e77c
Flags: needinfo?(felash)
Comment 52•12 years ago
|
||
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
NCK = Net Code Key
NSCK = Net Subset Code Key
CCK = Corporate Code Key
SPCK = Service Provider Code Key
Not sure if i have access to all these?
Comment 53•11 years ago
|
||
On AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.107, unlock screen willn't popup permanently. In ICCIO.cpp, we find card state is unknown. Attached file is log by QXDM. Please help to investigate.
Comment 54•11 years ago
|
||
SIM card state is unknown. There is no unlock screen popup.
Comment 56•11 years ago
|
||
Extremely sorry, this is a regression in commercial RIL. Identified and fixed the issue and it should be available in AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.116.
Flags: needinfo?(anshulj)
Comment 57•11 years ago
|
||
On AU121, unlock screen can popup and NW category single lock can be unlocked. But, NS/C/SP/SIM categories unlock screen will not popup. Please refer to log.
Comment 58•11 years ago
|
||
AU121 NS/SP/C/SIM lock will not popup
Comment 59•11 years ago
|
||
(In reply to buri.blff from comment #58)
> Created attachment 756483 [details]
> AU121 NS/SP/C/SIM lock will not popup
>
> AU121 NS/SP/C/SIM lock will not popup
Please use standard formats for attachments that way everyone can see the logs. I'm no able to see the log and therefore I cannot help here.
Flags: needinfo?(buri.blff)
Comment 60•11 years ago
|
||
Based on comment 57 & 58, the verification result is not OK ,so I reopen the Bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 62•11 years ago
|
||
batch update on tef+ milestones. partner to make a final on 6/3 Asia time. TEF+ needs to be resolved by 6/3 to be in the final build. thanks
Target Milestone: --- → 1.0.1 IOT3 (3jun)
Comment 63•11 years ago
|
||
Please do not reopen bugs. File new bugs as followups to the original issue. Reopening will mess up our landing process, so we need new bugs filed for followup bugs.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
Comment 64•11 years ago
|
||
Can you please provide steps to verify this fix - as we can perform black box testing from the UI?
Assignee | ||
Comment 65•11 years ago
|
||
You need a phone in a network locked state, when it starts, the NCK popup should appear as soon as possible.
Comment 66•11 years ago
|
||
We do not have access to a locked phone to be able to check this issue.
Whiteboard: QARegressExclude
Updated•11 years ago
|
Flags: needinfo?(buri.blff)
You need to log in
before you can comment on or make changes to this bug.
Description
•