[Flame] The SIM2's PIN1 code will not pop up when user only insert SIM card to slot 2.

VERIFIED FIXED in 2.2 S5 (6feb)

Status

defect
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: lixia, Assigned: yang.yang)

Tracking

unspecified
2.2 S5 (6feb)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 verified, b2g-v2.1S verified, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe])

Attachments

(12 attachments, 1 obsolete attachment)

166.37 KB, text/plain
Details
8.20 MB, video/mp4
Details
1.58 KB, patch
Details | Diff | Splinter Review
917 bytes, patch
apastor
: feedback+
Details | Diff | Splinter Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
apastor
: review+
Details | Review
46 bytes, text/x-github-pull-request
apastor
: review+
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
3.85 MB, video/mp4
Details
2.06 MB, video/mp4
Details
1.77 MB, video/mp4
Details
1.49 MB, video/mp4
Details
Posted file logcat_1535.txt
[1.Description]:
[Flame v2.1&v2.2][First Time Experience]The SIM2's PIN1 code will not pop up and no SIM2 signal on notifications bar when user only insert SIM card to slot 2 and then power on device.
Attch:SIM2_PIN1.mp4 and logcat_1535.txt
Found time:15:35

[2.Testing Steps]: 
1.Insert SIM2 card with PIN1 code to Slot2 (Slot 1 is empty).
2.Power on device.
3.Tap Settings->SIM manager->SIM Security.
4.Disable PIN1 code.
** SIM2 can be identified normally and call out successfully, the signal will display on notifications bar.

[3.Expected Result]: 
2.The SIM2's PIN1 code should pop up to prompt the user input PIN1 code.

[4.Actual Result]: 
2.The SIM2's PIN1 code will not pop up and no SIM2 signal shows on notifications bar.

[5.Reproduction build]: 
Flame 2.1 Build:
Gaia-Rev        97873dca486abf4162a3345e71b375806937bdec
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/9faa165ac85d
Build-ID        20141211001204
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141211.034507
FW-Date         Thu Dec 11 03:45:22 EST 2014
Bootloader      L1TC00011880

Flame 2.2 Build:
Gaia-Rev        1a956437210d2b16988d2ddbf40c9a38d8474435
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/d92db71d4e67
Build-ID        20141211040208
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141211.074025
FW-Date         Thu Dec 11 07:40:36 EST 2014
Bootloader      L1TC00011880

[6.Reproduction Frequency]: 
Always Recurrence,5/5
TCID:14367
(Reporter)

Comment 1

4 years ago
Posted video SIM2_PIN1.MP4
(Reporter)

Comment 2

4 years ago
[7.Note]
If user insert SIM1 card without PIN1 code and SIM2 card with PIN1 code, restart device, the SIM2's PIN1 code also doesn't pop up and only show SIM1 signal in status bar.
(Reporter)

Updated

4 years ago
Severity: normal → major
blocking-b2g: --- → 2.2+
Whiteboard: [systemsfe]
Guillaume, can you take a look here?
Flags: needinfo?(gmarty)

Comment 4

4 years ago
1007948 related also
(Assignee)

Comment 5

4 years ago
Alberto, I think this problem is happend by these changes below (for bug 1090859):

 // Always showing the first slot first.
- if (!this._alreadyShown && index > 1) {
+ if (!this._alreadyShown && index > 0) {
return false;
}

if there is only sim2 is locked, the sim2 unlock dialog will never open 
because the case (!this._alreadyShown && index > 0) is always be true.

I want to know for what reason you add the case (!this._alreadyShown && index > 0).

Thank you very much for your kindly reply.
Flags: needinfo?(apastor)
(Assignee)

Comment 6

4 years ago
Attachment #8543748 - Flags: review?(apastor)
(Assignee)

Comment 7

4 years ago
To solve this problem, i made some modify: 

(1)add the case for two slots both locked.

-      if (!this._alreadyShown && index > 0) {
+      var _isBothSlotsLocked = isBothSlotsLocked();
+      if (_isBothSlotsLocked && (!this._alreadyShown && index > 0)) {

(2)besides, let (this._alreadyShown = false;) at the head of init function,
because in the showIfLocked function, the flag this._alreadyShown might change to true.


Alberto, please review if my changes can slove this problem, thank you.
The particular modification can be find in the attachment no_sim2_unlock_dialog.patch.
Assignee: nobody → sfoster
assigning to yang.yang who has patch
Assignee: sfoster → yang.yang
Is this patch for 2.1? It seems that sim_lock.js doesn't exist anymore in master. Yang Yang, could you please provide a patch for master, and then we'll uplift it to 2.1? Let me know if you need any help.
Flags: needinfo?(apastor) → needinfo?(yang.yang)
Lock screen was refactored since 2.1, so we'll need 2.1 (if we are considering uplift) and 2.2/master specific patches. Cc-ing Greg Weng who has been dealing with this.
(Assignee)

Comment 11

4 years ago
(In reply to Alberto Pastor [:albertopq] from comment #9)
> Is this patch for 2.1? It seems that sim_lock.js doesn't exist anymore in master. 
 
  Yes, my previous patch is for 2.1.

> Yang Yang, could you please provide a patch for master, and then
> we'll uplift it to 2.1? Let me know if you need any help.

  I made a patch for master, please review it: no_sim2_unlock_dialog_master.patch.
Flags: needinfo?(yang.yang) → needinfo?(apastor)
(Assignee)

Comment 12

4 years ago
Attachment #8544952 - Flags: review?(apastor)
Comment on attachment 8544952 [details] [diff] [review]
patch for master

The patch looks good to me. I cannot review it, as I'm not a peer, so forwarding the review to :alive.

My only comment is that we should add some unit tests.

Thanks!
Flags: needinfo?(apastor)
Attachment #8544952 - Flags: review?(apastor)
Attachment #8544952 - Flags: review?(alive)
Attachment #8544952 - Flags: feedback+
Summary: [Flame][First Time Experience]The SIM2's PIN1 code will not pop up when user only insert SIM card to slot 2. → [Flame] The SIM2's PIN1 code will not pop up when user only insert SIM card to slot 2.
Comment on attachment 8544952 [details] [diff] [review]
patch for master

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

1. Avoid to create a new function inside a function; please move the isBothLocked function into SimLockManager.prototype.isBothLocked
2. Please create a github pull request
3. Please have a simple unit test in sim_lock_manager_test.js

Thank you!
Attachment #8544952 - Flags: review?(alive)
(In reply to Alberto Pastor [:albertopq] from comment #13)
> Comment on attachment 8544952 [details] [diff] [review]
> patch for master
> 
> The patch looks good to me. I cannot review it, as I'm not a peer, so
> forwarding the review to :alive.
> 
> My only comment is that we should add some unit tests.
> 
> Thanks!

You have my words to gain the ability to review sim lock stuff from now on :)
Flags: needinfo?(gmarty)
Comment on attachment 8543748 [details] [diff] [review]
no_sim2_unlock_dialog.patch

Please, flag me again after fix :alive's comments on master.

Thanks!
Attachment #8543748 - Flags: review?(apastor)
(Assignee)

Comment 17

4 years ago
Github pull-request:    https://github.com/mozilla-b2g/gaia/pull/27291

Alberto & Alive, please review it, thank you
Flags: needinfo?(apastor)
Flags: needinfo?(alive)
(Assignee)

Comment 18

4 years ago
(In reply to Yang Yang from comment #17)
> Github pull-request:    https://github.com/mozilla-b2g/gaia/pull/27291
> 
> Alberto & Alive, please review it, thank you

I am sorry, 27291 is closed

please review this one:
https://github.com/mozilla-b2g/gaia/pull/27292
Hi, please append a new request (or edit pr 27292's title and reopen the pr) and obsolete the old request. Also, alberto will review this from now on.
Flags: needinfo?(alive)
(Assignee)

Comment 20

4 years ago
Attachment #8547343 - Flags: review?(apastor)
Comment on attachment 8547343 [details] [review]
[master] pull request

The code looks good to me, but there are some Linting errors. Also, some unit tests are failing https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=bddda8608af7 (I think a isLocked() stub is missing). Please, reflag me after fixing those.

Thanks!
Flags: needinfo?(apastor)
Attachment #8547343 - Flags: review?(apastor)
(Assignee)

Comment 22

4 years ago
Posted file patch for master
Attachment #8552319 - Flags: review?(apastor)
(Assignee)

Comment 23

4 years ago
(In reply to Alberto Pastor [:albertopq] from comment #21)
> Comment on attachment 8547343 [details] [review]
> [master] pull request
> 
> The code looks good to me, but there are some Linting errors. Also, some
> unit tests are failing
> https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=bddda8608af7
> (I think a isLocked() stub is missing). Please, reflag me after fixing those.
> 
> Thanks!

albertopq, please review again, thank you
Comment on attachment 8552319 [details] [review]
patch for master

Good work! Just one more little thing. Can we add a simple test that checks isBothSlotsLocked logic? Right now, all the tests are mocking that method. Just trying to avoid changes to the slots affecting this. r=me as soon as that's added. Also, remember to set r=albertopq at the end of the pull request title.
Thanks!
Attachment #8552319 - Flags: review?(apastor) → review+
(Assignee)

Updated

4 years ago
Attachment #8553567 - Flags: review?(apastor)
Comment on attachment 8553567 [details] [review]
[master] pull request:  https://github.com/mozilla-b2g/gaia/pull/27621

Great job! Thanks!
Attachment #8553567 - Flags: review?(apastor) → review+
master: https://github.com/mozilla-b2g/gaia/commit/df9f4f23147c8c9adbcf2ff5c0a065ab408b34ee
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Kevin, do we want this for 2.1?
Flags: needinfo?(khu)
Comment on attachment 8553567 [details] [review]
[master] pull request:  https://github.com/mozilla-b2g/gaia/pull/27621

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Bug 1090859.

[User impact] if declined:
Disabling SIM2's pin when booting device doesn't work.

[Testing completed]:
Manually tested, added unit tests.

[Risk to taking this patch] (and alternatives if risky):
This patch effects how we display the SIM pin dialog when booting the phone, and makes sure we show the SIM pin in more cases. If anything it is more risky not to accept this patch.

[String changes made]: none.
Attachment #8553567 - Flags: approval-gaia-v2.2?
Attachment #8553567 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
(Assignee)

Comment 31

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #30)
> v2.2:
> https://github.com/mozilla-b2g/gaia/commit/
> 9bc5a10d2fc2f472ffeba061b8b47a5523f6b004

Because 2.1 has the same problem, we also want this for 2.1
(In reply to Yang Yang from comment #31)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #30)
> > v2.2:
> > https://github.com/mozilla-b2g/gaia/commit/
> > 9bc5a10d2fc2f472ffeba061b8b47a5523f6b004
> 
> Because 2.1 has the same problem, we also want this for 2.1

Please request 2.1 uplift approval, like I did in comment 29 for 2.2.
Flags: needinfo?(yang.yang)
(Assignee)

Comment 33

4 years ago
Comment on attachment 8553567 [details] [review]
[master] pull request:  https://github.com/mozilla-b2g/gaia/pull/27621

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Bug 1090859.

[User impact] if declined:
Disabling SIM2's pin when booting device doesn't work.

[Testing completed]:
Manually tested, added unit tests.

[Risk to taking this patch] (and alternatives if risky):
This patch effects how we display the SIM pin dialog when booting the phone, and makes sure we show the SIM pin in more cases. If anything it is more risky not to accept this patch.

[String changes made]: none.
Flags: needinfo?(yang.yang)
Attachment #8553567 - Flags: approval-gaia-v2.1?
Attachment #8553567 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Needs a rebasing for v2.1 uplift.
Flags: needinfo?(yang.yang)
Comment on attachment 8560228 [details] [review]
[PullReq] YangYangSPRD:bug1110660_v2.1 to mozilla-b2g:v2.1

This has Gaia unit test failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=498982&repo=gaia-try
Attachment #8560228 - Flags: feedback-
(Assignee)

Comment 37

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #36)
> Comment on attachment 8560228 [details] [review]
> [PullReq] YangYangSPRD:bug1110660_v2.1 to mozilla-b2g:v2.1
> 
> This has Gaia unit test failures.
> https://treeherder.mozilla.org/logviewer.html#?job_id=498982&repo=gaia-try

I am working for it now。
Flags: needinfo?(yang.yang)
Attachment #8560228 - Attachment is obsolete: true
This bug has been successfully verified on latest Flame v2.2&3.0.
See attachment: verified_v2.2_(1).mp4 and verified_v2.2_(2).mp4
Reproduce rate: 0/5.

Flame 2.2 build:
Build ID               20150209002504
Gaia Revision          e827781324cbde91d2434b388f5dead3303a85ee
Gaia Date              2015-02-06 20:54:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0552759956d3
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150209.040038
Firmware Date          Mon Feb  9 04:00:51 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 :
Build ID               20150209010211
Gaia Revision          0d7b35f23402c4cb29bca6b98280fec48a196dec
Gaia Date              2015-02-08 20:44:20
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3436787a82d0
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150209.045655
Firmware Date          Mon Feb  9 04:57:06 EST 2015
Bootloader             L1TC000118D0
This bug has been successfully verified on latest 2.1S and Flame v2.1.
See attachment: verified_v2.1_(1).mp4 and verified_v2.1_(2).mp4
Reproduce rate: 0/5.

Flame 2.1 build:
Build ID               20150210002200
Gaia Revision          7dd130a312f12c89b2d41948f8557effa56afbf6
Gaia Date              2015-02-10 06:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/2de03dfa9aac
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150210.040336
Firmware Date          Tue Feb 10 04:03:47 EST 2015
Bootloader             L1TC000118D0

2.1S (256m):
Build ID               20150210002201
Gaia Revision          7dd130a312f12c89b2d41948f8557effa56afbf6
Gaia Date              2015-02-10 06:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/5017c32553c7
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150210.040821
Firmware Date          Tue Feb 10 04:08:33 EST 2015

2.1S (512m):
Build ID               20150210161202
Gaia Revision          7dd130a312f12c89b2d41948f8557effa56afbf6
Gaia Date              2015-02-10 06:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/df6f80e36912
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  122
Firmware Date          Thu Feb  5 12:42:58 CST 2015
Status: RESOLVED → VERIFIED
clear ni because it's in 2.1.
Flags: needinfo?(khu)
You need to log in before you can comment on or make changes to this bug.