Closed Bug 905173 Opened 6 years ago Closed 6 years ago

PIN code screen should not display "3 tries left." when loading

Categories

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

defect
Not set

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

RESOLVED FIXED
1.2 C3(Oct25)
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: rik, Assigned: fcampo)

References

Details

Attachments

(1 file)

"3 tries left." is briefly displayed at every device launch before disappearing. We should never display it when there is no error.
So this id=triesLeft element showing up. We could just put hidden=true in the markup to fix this.

But I dig up a bit and IccHelper.getCardLockRetryCount() seems to always returns 0. Hence id=triesLeft is never shown. Instead, when we receive a PIN code error, we display id=errorMsg.

This was introduced in bug 860660. Fernando, could we remove the whole triesLeft logic here or am I missing something?
Flags: needinfo?(fernando.campo)
Assignee: nobody → fernando.campo
Flags: needinfo?(fernando.campo)
Bug confirmed on b2g18 and moz-central: IccHelper.getCardLockRetryCount() always returns 0.

This can be quite serious in some scenarios, e.g.:
 • a user tries to enable the SIM security and does two errors ⇒ gets scared and quits the Settings app;
 • later, having forgotten that two attempts have been tried, gets back to the Settings app and tries to enable the SIM security again ⇒ no warning is displayed ⇒ risk of blocking.
blocking-b2g: --- → koi?
blocking-b2g: koi? → koi+
(In reply to Anthony Ricaud (:rik) from comment #1)
> So this id=triesLeft element showing up. We could just put hidden=true in
> the markup to fix this.
I think this is the best option

> But I dig up a bit and IccHelper.getCardLockRetryCount() seems to always
> returns 0. Hence id=triesLeft is never shown. Instead, when we receive a PIN
> code error, we display id=errorMsg.
IccHelper returning always 0 seems like a bug.

> This was introduced in bug 860660. Fernando, could we remove the whole
> triesLeft logic here or am I missing something?
As Kaze states on comment 2, the logic behind showing the number of retries when loading the screen is needed to avoid blocking the phone.

So my proposal here is to hide by default the number of retries, and show it only if it's > 0, then open a new bug about IccHelper returning always 0.

I understand also that having the number of retries shown in two different places is a mistake, but that is more a UX thing, and probably will be dealt with on bug 852682.
Depends on: 924949
I was going to put Kaze as reviewer, but saw that his list of pending reviews is longer than my shopping list :D
Rik, if you can't review this, blank the r and I'll try to re-assign (but honestly I think it's a quick one)

Cheers!
Attachment #814943 - Flags: review?(anthony)
Comment on attachment 814943 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12758

OK, I understood the distinction between triesLeft (displayed when arriving on the screen) and errorMsg (displayed after a user submits a code).

I've seen one logic error that I commented in the PR.

Can you adapt the HTML in the tests and probably add new tests?
Attachment #814943 - Flags: review?(anthony) → review-
Comment on attachment 814943 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12758

Good catch with the attribute!

I should change all the class='hidden' for attributes, to be honest, but we can do that on a polishing bug

Also, tests fixed
Attachment #814943 - Flags: review- → review?
Comment on attachment 814943 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12758

Just realized that I didn't assign the review, sorry
Attachment #814943 - Flags: review? → review?(anthony)
Comment on attachment 814943 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/12758

Excellent, thank you very much!
Attachment #814943 - Flags: review?(anthony) → review+
master: 43091d1a13e4bbb2693f5238c80307fca36e2175

Thanks!
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment mime type: text/plain → text/x-github-pull-request
I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 43091d1a13e4bbb2693f5238c80307fca36e2175
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(fernando.campo)
uplifted to 1.2 91b8ead867fd68bbe30ded6a8aa8861b95f4c536
Flags: needinfo?(fernando.campo)
Target Milestone: --- → 1.2 C3(Oct25)
You need to log in before you can comment on or make changes to this bug.