[first run] on last attempt for correct PUK, two messages with same meaning shown/concatenated

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: aryx, Assigned: pasupulaphani, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe][good first bug])

Attachments

(2 attachments, 1 obsolete attachment)

Boot2Gecko 1.2.0.0-prerelease 20131104224010 on Keon, update channel 'default'

When running the phone for the first time with a locked SIM card and entering the wrong PUK code for unlocking until you get to the last attempt, the message below the input field to enter the PUK code has twice the same message because the strings pukAttemptMsg2[one] and pukLastChanceMsg are shown. Only one should be used.
Furthermore, they have been concatenated without using a whitespace in between.
Can you attach a screenshot so we can see the severity of the problem?
Flags: needinfo?(archaeopteryx)
Whiteboard: [systemsfe]
Created attachment 8498200 [details]
screenshot of issue

(In reply to Michael Henretty [:mhenretty] from comment #2)
> Can you attach a screenshot so we can see the severity of the problem?
The severity is very low to non-existent.
Flags: needinfo?(archaeopteryx)
Agreed, we only need to show one message.
Mentor: mhenretty
Whiteboard: [systemsfe] → [systemsfe][good first bug]
(Assignee)

Comment 5

4 years ago
Created attachment 8515498 [details] [review]
This fix will stop multiple error messages to appear.
Attachment #8515498 - Flags: review?(fernando.campo)
Comment on attachment 8515498 [details] [review]
This fix will stop multiple error messages to appear.

Patch does the work (nice addition to include the rest of the cases apart from PUK), just a small nit using the occasion that we're dealing with the 'retryCount'.


But even though the fix does the work, I think that the problem in here is deeper than showing one message or the other. The different kind of errors are separated for a reason, one dealing with the incorrect code introduced (and the chances left), and the other with the extra danger on the last attempt. The fact that the message in both is very similar is, on my opinion, a mistake (but probably should be fixed in a different bug). If we are gonna show one message or the other, we can deal with that just changing the message, not showing/hiding different DOM elements.

So I'm happy with this fix if it solves the issue, but would recommend a second look from UX / rewording.
Flags: needinfo?(jsavory)
Attachment #8515498 - Flags: review?(fernando.campo) → review+
Thanks for updating the PR Phani.

Now that the the patch is reviewed, we need to prepare it for merge:
1. Squash the 2 commits of the patch into a single commit. This helps with a better readable commit history for the project.
2. Give the commit a better message, describing which bug are you solving (usually just copying the whole name of the bug, or shortening it if too long) 
e.g. "Bug 936823 - [FTE] on last attempt for correct PUK, two messages with same meaning shown/concatenated"
This helps when searching through the commit history
3. Add the reviewer nickname at the end of the commit message "(r=fcampo)". This way we know who to extra blame if something goes wrong :D

You can check the link https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Submitting_a_Gaia_patch#Patch_submission for instructions if needed.
Flags: needinfo?(pasupulaphani)
(Assignee)

Comment 8

4 years ago
Cheers for the info.

I've squashed them to 1 commit and updated commit message.
Flags: needinfo?(pasupulaphani)
Hi Phani, thanks for your help here.

I still see 4 commits in https://github.com/mozilla-b2g/gaia/pull/25724. Perhaps you opened a new pull request with 1 commit?
Assignee: nobody → pasupulaphani
Flags: needinfo?(pasupulaphani)
(Assignee)

Comment 10

4 years ago
Hi Michael, I haven't done this before. For some reason it is generating a new merged commit!

I'm not sure what I'm doing wrong, I will try again.
Flags: needinfo?(pasupulaphani)
(In reply to Phani from comment #10)
> Hi Michael, I haven't done this before. For some reason it is generating a
> new merged commit!
> 
> I'm not sure what I'm doing wrong, I will try again.

Try rebasing against the master branch in interactive mode:
`git rebase -i master`

Then, in the editor, change all the commmit actions after the first one from 'pick' to 'squash'. That should give you one commit.
(Assignee)

Comment 12

4 years ago
Created attachment 8519412 [details] [review]
Pull request with 1 commit
Attachment #8515498 - Attachment is obsolete: true
Good job Phani! Thanks for you help :)

master: https://github.com/mozilla-b2g/gaia/commit/777ceb12bfcd7baadf0f176df467bf6a1960f9e3
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Agree as well that only one message is necessary. Flagging Jenny to look at the wording as I believe she has worked on the SIM/PUK Pin pages.
Flags: needinfo?(jsavory) → needinfo?(jelee)

Comment 15

4 years ago
Hi there, current spec string is as follow: 
"Incorrect PUK. You have %num tries left to enter the correct code before the SIM card becomes permanently unusable."
Thanks!
Flags: needinfo?(jelee)
You need to log in before you can comment on or make changes to this bug.