[FDN] Incorrect message displayed when updating FDN contact

VERIFIED FIXED in 2.2 S9 (3apr)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: anshulj, Assigned: eragonj)

Tracking

({regression})

unspecified
2.2 S9 (3apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.0 unaffected, b2g-v2.1 affected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [caf priority: p2][CR 811090], )

Attachments

(6 attachments)

Reporter

Description

4 years ago
STR

1. Go to call settings -> Fixed Dialing Numbers and click on Authorized numbers setting.
2. Click add and enter a name and number for a contact to add.
3. Hit ok and enter an incorrect PIN2
4. Repeat step 3 until you are asked for PUK2 at which time, enter PUK2 and new PIN2s.
5. Repeat steps 2 and 3

Expected: After entering an incorrect PIN2 on step 3 the retry count for PIN2 should be displayed

Observed: PIN2 retry count is displayed but in addition there is a mention of entering PUK2. Please see attachment. In other case I saw a mention of 2 retries left and a last retry left on the same screen.
Reporter

Comment 1

4 years ago
Reporter

Comment 2

4 years ago
Reporter

Updated

4 years ago
Summary: Incorrect message displayed when updating FDN contact → [FDN] Incorrect message displayed when updating FDN contact
This was found by Anshul when testing bug 1020757, let's see what I can fix here, set myself as assignee first.
Assignee: nobody → ejchen
Whiteboard: [CR 811090]
Whiteboard: [CR 811090] → [caf priority: p2][CR 811090]

Comment 4

4 years ago
Triage: blocking
blocking-b2g: 2.2? → 2.2+

Comment 5

4 years ago
(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #3)
> This was found by Anshul when testing bug 1020757, let's see what I can fix
> here, set myself as assignee first.

Hi EJ,

How are you progressing with this? Will you be able to deliver a fix for this by the fxOS 2.2 FC date of April 6th?

Thanks,
Mike
Flags: needinfo?(ejchen)
Hi Mike,

I just started to investigate this because bug 1020757 just landed yesterday. I'll check the root cause first and update later about the estimated date for this patch.

THanks.
Flags: needinfo?(ejchen)
Hi Anshul,

based on this comment (https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/settings/js/simcard_dialog.js#L275) from Sergi, we can't get the retryCount directly from Gecko because there is no retryCount info from the error object. 

How do you show that UI when pressing the the wrong PIN2 ? After reading the codes and doing some manual tests, we can't show the retryCount here. Did you guys update the source code from Gaia & Gecko for this part ?
Flags: needinfo?(anshulj)
Reporter

Comment 8

4 years ago
Ej Chen, no we haven't modified the Gecko/Gaia code for this part at all. Could you please try to reproduce the issue yourself?
Flags: needinfo?(anshulj)
Eric, can you help me to verify this bug (?) I can't reproduce it on 2.2 & master. Do you know how to reproduce it easily ? Thanks :)
Flags: needinfo?(echang)
Per offline talk, it seems to be different sim cards have different output, please use mine for testing, thanks.
Flags: needinfo?(echang)
Anshal, we can't reproduce that, can you check the youtube link and see what's the missing step compared with yours ? Thanks.
Flags: needinfo?(anshulj)

Comment 12

4 years ago
Can QA reproduce it?
Flags: needinfo?(echang)
Keywords: qawanted
There are only two places that would show retryCount, one is https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/settings/js/simcard_dialog.js#L464 while another one is https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/settings/js/simcard_dialog.js#L146

After investigating, the first link would be the most possible place that would trigger and show related information but if you check the comments above that line, you will notice that this would only be used in emulator.

For another one, because our entry point is 'updateFDN', there is no way to enter this part of code based on your STR.

I suspect that there might be some missing steps or the "should-not-enter" code was executed accidentally (if this is the case, the first entry point might be the most suspicious one). 

Hope this information helps.
No, but I can reproduce with adding the 'updateFDN' step(as described in comment 13) which is not the same as comment 0. 
(In reply to howie [:howie] from comment #12)
> Can QA reproduce it?
Flags: needinfo?(echang)
This issue is actually slightly different than described in comment 0.  The page in question, SIM Pin 2 was incorrectly entered when adding/editing an FDN contact, will show extra text depending on the last error message seen.  

For example, if no errors have been encountered while getting to that page, the only message seen will be that the PIN 2 code is incorrect.  If the user also fails to input the correct PIN while enabling FDN the last seen retry count message (either 2 or last will be shown, but it will show the same for every failed entry, not the correct one) will have its text shown.  If the user fails 3 times and sees the PUK2 page, they will see text from the PUK2 entry screen.
	
I am attaching a screenshot of these 3 situations and putting new steps below.

Prerequisites: 
1) Know the PUK2 code of the PIN you are using.

Repro Steps:
1) Update a Flame to 20150331054712
2) Go to Settings > Call Settings > Fixed Dialing Numbers (make sure to use the correct PIN on the first step)
3) Enable the FDN
4) Go to Authorized numbers > Add a number
5) Input the incorrect PIN and note the error screen
6) Return to FDN main menu and attempt to disable FDN with the wrong PIN
7) Repeat steps 4 and 5 noting the new text on the error screen
8) Fail to enter the PIN until asked to unlock with a PUK2 code
9) Fail to enter the correct PIN one final time and note the new text on the error screen

This issue does occur on Flame 3.0, 2.2, and 2.1.

Environmental Variables:
Device: Flame Master
BuildID: 20150331054712
Gaia: 1d14f4a6809de81ebd638117ed4ddd3b1b18f033
Gecko: c20f8549d631
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 2.2
BuildID: 20150331095103
Gaia: ca32692cd4c1bb98f3878268c3234d84716ee8a9
Gecko: 365147b2477c
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1
BuildID: 20150331081606
Gaia: 63c89e001010a9a43ea55afd39333ded3d5e83fc
Gecko: 30ac9a03343a
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

This issue does NOT occur on Flame 2.0.

Environmental Variables:
Device: Flame 2.0
BuildID: 20150330073217
Gaia: 896803174633fc6acd3fd105f81c349b8e9b9633
Gecko: 9c12f28cc73f
Version: 32.0 (2.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: qawantedregression
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Reporter

Updated

4 years ago
Flags: needinfo?(anshulj)
QA Contact: jmercado
This issue first began occurring during a period when there was a bug that caused a black screen to appear on tinderbox builds.  This caused my b2g-inbound window's pushlog to be larger than my central window's pushlog.  As such I have included the central window instead.  I am not sure which bug is the cause of this issue at all.

Central Regression Window:

Last Working 
Environmental Variables:
Device: Flame 2.1
BuildID: 20140731150609
Gaia: 04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908
Gecko: 104254bd1fc8
Version: 34.0a1 (2.1) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken 
Environmental Variables:
Device: Flame 2.1
BuildID: 20140801070614
Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824
Gecko: 8970589d6505
Version: 34.0a1 (2.1) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Last Working gaia / First Broken gecko - Issue does NOT occur
Gaia: 04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908
Gecko: 8970589d6505

First Broken gaia / Last Working gecko - Issue DOES occur
Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824
Gecko: 104254bd1fc8

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908...9689218473b6fc4dd927ad6aa7b06c05f0843824
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
It seems that the original STR missed a critical step to reproduce the bug, originally, what QA saw would be another bug for me, but I am fine to use this bug to fix that if we confirmed that what Anshul want to report is the same with what we found here.

So now, I just provided two solutions for our QA found and will start the review process with module owner.
Comment on attachment 8586600 [details] [review]
[gaia] EragonJ:bug-1145332 > mozilla-b2g:master

Arthur, can you help me check this patch ? Thanks !!
Attachment #8586600 - Flags: review?(arthur.chen)
Comment on attachment 8586605 [details] [review]
[gaia] EragonJ:v2.2-bug-1145332 > mozilla-b2g:v2.2

Arthur, can you help me check this patch ? Thanks !!
Attachment #8586605 - Flags: review?(arthur.chen)
Comment on attachment 8586600 [details] [review]
[gaia] EragonJ:bug-1145332 > mozilla-b2g:master

Thanks, EJ!
Attachment #8586600 - Flags: review?(arthur.chen) → review+
Attachment #8586605 - Flags: review?(arthur.chen) → review+
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 8586605 [details] [review]
[gaia] EragonJ:v2.2-bug-1145332 > mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no
[User impact] if declined: users will see wrong information when following STR
[Testing completed]: yes 
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: no
Attachment #8586605 - Flags: approval-gaia-v2.2?
Autolander landed this on v2.2.
v2.2: https://github.com/mozilla-b2g/gaia/commit/1ceca464053dee4a8bf10ea5abeef724d68c2ff2

Bhavana, should we just go for retroactive approval at this point?

Kevin, this isn't the first time. Can we please make Autolander stop ignoring the lack of approvals on branch landings?
Flags: needinfo?(kgrandon)
Flags: needinfo?(bbajaj)
Target Milestone: --- → 2.2 S9 (3apr)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #26)
> Kevin, this isn't the first time. Can we please make Autolander stop
> ignoring the lack of approvals on branch landings?

I've filed bug 1150080 to implement this. It would be good to find an interim solution, but I can't think of a good one besides only starting with a single pull request against master.
Flags: needinfo?(kgrandon)
Jayme, can you verify that this issue is fixed?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(jmercado)
Keywords: verifyme
Oooops, sorry Ryan & Kevin, I didn't notice that autolander would directly land patches into non-master branch. I'll try to avoid using our autolander until related bug is fixed. Thanks.
Hi Anshul, I am not sure what's the real case you are going to report, for a heads up, with your STR, we can't reproduce anything, but we found something else with different STR, because I tried to reach to you to make sure whether this is what you want to report but you didn't respond, we decided to fix what we found here.

So, If you found something else, please file another bug for this. Otherwise, I think this would be the patch that I can deliver for now. Thanks.
Flags: needinfo?(anshulj)
Reporter

Comment 31

4 years ago
EJ, let me test your patch today with both your STRs and mine and provide you feedback.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #26)
> Autolander landed this on v2.2.
> v2.2:
> https://github.com/mozilla-b2g/gaia/commit/
> 1ceca464053dee4a8bf10ea5abeef724d68c2ff2
> 
> Bhavana, should we just go for retroactive approval at this point?
yea, that's fine at this point :|

> Kevin, this isn't the first time. Can we please make Autolander stop
> ignoring the lack of approvals on branch landings?
Flags: needinfo?(bbajaj)
Attachment #8586605 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Reporter

Comment 33

4 years ago
Comment on attachment 8586605 [details] [review]
[gaia] EragonJ:v2.2-bug-1145332 > mozilla-b2g:v2.2

EJ, I am still able to reproduce the issue with the STRs mentioned in comment#0. I validated that this patch fixes the issue.
Flags: needinfo?(anshulj)
Attachment #8586605 - Flags: feedback+
Thanks Anshul for your manual validation ! I really appreciate it.
I was able to verify that this issue works just fine with the latest Nightly Flame 2.2 build, but verifying this issue is blocked by bug 1151105.  Leaving the ni? on myself to check this issue when bug 1151105 is fixed.

Actual Results: The error message does not have extra text from other error messages.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150403002503
Gaia: 022eeb91197ba4a9adfd67bd6db5aa03cc69eb31
Gecko: 77fdc6fbcae9
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
This issue is not fixed on the latest Nightly 3.0 build

Actual Results: The user is shown incorrect error messages when failing to enter the SIM pin 2 on the authorized numbers page.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(jmercado) → needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
This failed verification, removing verifyme since it failed verify. NI :kgrandon to get attention.
Flags: needinfo?(kevingrandon)
Keywords: verifyme
(In reply to Pi Wei Cheng [:piwei] from comment #37)
> This failed verification, removing verifyme since it failed verify. NI
> :kgrandon to get attention.

I can't do too much here, and this feels like something in Settings? Tim - can you have someone take a look here?
Flags: needinfo?(kevingrandon) → needinfo?(timdream)
Flags: needinfo?(timdream) → needinfo?(gasolin)
Does the issue related to the bug 1205596?
Flags: needinfo?(gasolin) → needinfo?(pcheng)
Found the PR was not merged to master, cherry-pick and rebased the PR. The patch looks no harm so I'd carry arthur's r+ and land it if treeherder green.
QAwanted to address comment 39
Keywords: qawanted
I do believe that this issue is the same as bug 1205596.  Leaving the qawanted tag to check both issues once we have a master build with this patch applied.
This issue is verified fixed on Flame 2.5.
Result: The correct message string is now displayed when entering the inccorect PUK2 code.

Environmental Variables:
Device: Flame 2.5 Kk Fullflash (319mb)
Build ID: 20151002030232
Gaia: 9a682cb7bc8b7fde624a9b2b3c2d64415a08b04b
Gecko: 5f16c6c2b969f70e8da10ee34853246d593af412
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 44.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][failed-verification] → [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(pcheng) → needinfo?(jmercado)
Keywords: qawanted
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.