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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Furthermore, they have been concatenated without using a whitespace in between.

Comment 2

[Michael Henretty [:mikehenrty][:mhenretty]]

Can you attach a screenshot so we can see the severity of the problem?

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: 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.

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

Agreed, we only need to show one message.

Comment 5

[Phani]

Attachment: This fix will stop multiple error messages to appear.

Comment 6

[Fernando Campo (:fcampo)]

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.

Comment 7

[Fernando Campo (:fcampo)]

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.

Comment 8

[Phani]

Cheers for the info.

I've squashed them to 1 commit and updated commit message.

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

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?

Comment 10

[Phani]

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.

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

(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.

Comment 12

[Phani]

Attachment: Pull request with 1 commit

Comment 13

[Michael Henretty [:mikehenrty][:mhenretty]]

Good job Phani! Thanks for you help :)

master: https://github.com/mozilla-b2g/gaia/commit/777ceb12bfcd7baadf0f176df467bf6a1960f9e3

Comment 14

[Jacqueline Savory [:jsavory] UX]

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.

Comment 15

[Jenny Lee]

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!