Closed Bug 1020757 Opened 5 years ago Closed 5 years ago

[FDN] Enable/Disable FDN asks to enter PUK if the enable/disable FDN failed

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S9 (3apr)
feature-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: anshulj, Assigned: eragonj)

References

Details

(Keywords: late-l10n, Whiteboard: [caf priority: p2][CR 725348][ft:comms] [p=12])

Attachments

(4 files)

STR.
1. Go to Call Settings->Fixed Dialing Number
2. Enable FDN
3. If the ICC card doesn't support FDN an error is returned when enabling FDN

Expected: An error message in displayed that FDN failed to enable/disable
Observed: PUK UI is displayed instead.
The root cause of the issue is that handleCardLockError function at http://lxr.mozilla.org/gaia/source/apps/settings/js/simcard_dialog.js#97 assumes that the call to enable/disable will always provide a retry count left. For errors such as GenericFailure  at http://lxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_consts.js#226 the retry count is unavailable.
In short we need to do something like handleMessage at http://androidxref.com/4.4.2_r2/xref/packages/services/Telephony/src/com/android/phone/FdnSetting.java#244
Does this happen on 1.4?
Keywords: qawanted
(In reply to Jason Smith [:jsmith] from comment #3)
> Does this happen on 1.4?
Yes it does. This is not a regression but a broken feature all along.
Keywords: qawanted
This is a long-standing issue that wasn't considered a certification issue when FDN shipped in a release, so we don't need to block on this.
blocking-b2g: 2.0? → backlog
Was discussed offline with ikumar/Anshul to consider this a waiver for 2.0 and help get a fix for 2.1.

Tim, per offline discussion with Anshul he explained this should be a simple change, can you comment here and help get this fixed for 2.1 ?
Flags: needinfo?(timdream)
This is a comms team feature.
Flags: needinfo?(timdream) → needinfo?(dscravaglieri)
Whiteboard: [ft:comms]
Sergi, could you take a look at this please ?
Assignee: nobody → sergi.mansilla
Flags: needinfo?(dscravaglieri)
Blocks: CAF-v2.1-FC-metabug
No longer blocks: CAF-v2.0-FC-metabug
Target Milestone: --- → 2.0 S6 (18july)
Component: Gaia::Settings → Gaia::Contacts
I made a fix but I need to test it. The problem is that all the SIM cards I have accept FDN. Is there any trick I can do to simulate that the SIM card doesn't accept FDN?
Flags: needinfo?(anshulj)
Sergi, you would have to hack your code to reproduce this scenario. If you upload your patch, I can give it a shot.
Flags: needinfo?(anshulj)
Hi Anshul,

My branch is at https://github.com/sergi/gaia/tree/BUG_1020757. The fix is simple, I believe. I check whether retryCount exists, and if not I inform the user and exit the function, without getting in unlocking dialogs.

Could you also paste the error that you get when trying enabling FDN in your device?

Thanks!
Flags: needinfo?(anshulj)
Sergi, the change you made would work but is still error prone because the code is relying on retry count instead of error messages returned by RIL. If count is less than zero then RIL already returns as error message that PUK is required which telephony sends it up as "SimPuk2" so the code doesn't need to check if count is less that 0. Similarly if the password is incorrect RIL already sends an error "IncorrectPassword" and so on. 

So my suggestion was to look at event.errorMsg instead of retry count. Something like below

  // card lock error messages
  function handleCardLockError(event) {
    var type = event.lockType; // expected: 'pin', 'fdn', 'puk'
    if (!type) {
      skip();
      return;
    }

    if (event.errorMsg === 'SimPuk2') {
      _action = initUI('unlock_puk2');
      pukInput.focus();
    } else if (event.errorMsg === 'IncorrectPassword') {
      var msgId = (count > 1) ? 'AttemptMsg3' : 'LastChanceMsg';
      showMessage(type + 'ErrorMsg', type + msgId, { n: count });
      showRetryCount(count);
    } else {
      var msgId = (count > 1) ? 'AttemptMsg3' : 'LastChanceMsg';
      // FdnNotSupported should be more generic like "FDN operation failed"
      showMessage('fdnNotSupported', type + msgId, { n: count });
      showRetryCount(count);
    }

    if (type === 'pin' || type === 'fdn') {
      pinInput.focus();
    } else if (type === 'puk') {
      pukInput.focus();
    }
  }

This way we would be able to add more specific messages in future. Please let me know what you think? I see similar issue when trying to edit an FDN on a SIM card that doesn't have FDN available. The code at http://lxr.mozilla.org/gaia/source/apps/settings/js/simcard_dialog.js#241 only handles the password incorrect error. For other cases I think we should display a generic error (perhaps fdnNotSupported) instead of just throwing an error that only gets printed in the logs.
Flags: needinfo?(anshulj)
Hi Anshul,

Could you please give it another try with your FDN-disabled SIM card? I have updated the branch.

Thanks!
Flags: needinfo?(anshulj)
Sergi, there are a bunch of patches on your branch and am not sure which ones to pick. Wondering if you could provide one consolidated patch for me to try inorder to make sure I am testing what you want me to?
Flags: needinfo?(anshulj)
Target Milestone: 2.0 S6 (18july) → 2.1 S1 (1aug)
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
Hi Anshul,

I've unified the patch, and you cna find it all in the commit https://github.com/sergi/gaia/commit/eb4e984cfb2333db7673809484c37047b4aeb0fe. Let me know via needinfo if you need anything else.

Thanks!
Flags: needinfo?(anshulj)
Sorry for the delay Sergi; busy with commercialization of 2.0. Will get back to you in a day or two.
Sergi, I left some comments for you on github.
Flags: needinfo?(anshulj)
Target Milestone: 2.1 S2 (15aug) → ---
Target Milestone: --- → 2.1 S3 (29aug)
Anshul, left some comments in the Github PR. Getting there!
Flags: needinfo?(anshulj)
Sergi, replied to your comments on github. Let me test your change to see how it behaves but I am hoping you can do some of the tests too on a device yourself to augment my testing.
Flags: needinfo?(anshulj)
Target Milestone: 2.1 S3 (29aug) → 2.1 S4 (12sep)
Needinfo to notify the ETA changes.(see whiteboard)
Please be noted that also depends on the cadence of testing feedback.
Flags: needinfo?(anshulj)
Whiteboard: [ft:comms] → [ft:comms] [ETA:9/19]
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
Thanks Wesley.
Flags: needinfo?(anshulj)
feature-b2g: --- → 2.1
Hi Anshul,

My latest changes are in. I will try to find a card with which I can test all the scenarios, but in the meantime perhaps you can give it another try. I expect to have the card soon this week.
Flags: needinfo?(anshulj)
Whiteboard: [ft:comms] [ETA:9/19] → [CR 725348][ft:comms] [ETA:9/19]
Whiteboard: [CR 725348][ft:comms] [ETA:9/19] → [caf priority: p2][CR 725348][ft:comms] [ETA:9/19]
blocking-b2g: backlog → ---
Hi Anshul,

The patch seems to work alright, but I don't think I can find a SIM card that doesn't have FDN capability. I need you to test my branch https://github.com/sergi/gaia/tree/BUG_1020757 with your card, or in case you know, I need you to tell me how to emulate a SIM card with FDN disabled.

Cheers,
Sergi
Right now, the assumption that FDN is disabled given certain conditions happens here: https://github.com/mozilla-b2g/gaia/pull/24299/files#diff-a888a0705d0739307eaed9b0bef204bdR152
Merged at e1fd99454b6cd5da4f2c58f928fc04c6d03f478f
(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #25)
> Merged at e1fd99454b6cd5da4f2c58f928fc04c6d03f478f

Please disregard that comment. It concerns another bug.
(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #23)
> Hi Anshul,
> 
> The patch seems to work alright, but I don't think I can find a SIM card
> that doesn't have FDN capability. I need you to test my branch
> https://github.com/sergi/gaia/tree/BUG_1020757 with your card, or in case
> you know, I need you to tell me how to emulate a SIM card with FDN disabled.
> 
> Cheers,
> Sergi
Sergi, first of all sorry for all the delay, it's been really busy at work. I left some comments for you on github. I tested your patch for the case where genericerror is returned while enabling FDN. I now don't see the PukRequired UI, the original issue reported in this bug. However I see the following.

1. I don't see a dialog message "An unknown error occured" but only see the console.error message on line #156.
2. Right after the error, the PIN2 UI is closed, as expected, and I see two consecutive requests for card lock state from Gaia for FDN. Not sure if this is needed.
Flags: needinfo?(anshulj)
Target Milestone: 2.1 S5 (26sep) → 2.1 S6 (10oct)
Sergi, this is expected to land before FC although, we are too late in 2,1 to be taking the features now, can you confirm this is going to make it ?

anshul, hoe critical is this to have in 2.1 ? Is there a chance we can accommodate this in future based on sergi's respones?
Flags: needinfo?(sergi.mansilla)
Flags: needinfo?(anshulj)
Bhavana, this is not critical for 2.1.
Flags: needinfo?(anshulj)
I expect to have a candidate fix at the end of today, CET.
Flags: needinfo?(sergi.mansilla)
Whiteboard: [caf priority: p2][CR 725348][ft:comms] [ETA:9/19] → [caf priority: p2][CR 725348][ft:comms]
The fix is there, but unfortunately this needs a lot of testing. I will need more time to write unit tests, 2-3 days.
Whiteboard: [caf priority: p2][CR 725348][ft:comms] → [caf priority: p2][CR 725348][ft:comms] [p=12]
Target Milestone: 2.1 S6 (10oct) → 2.1 S7 (24Oct)
Attached file Github PR
Attachment #8506085 - Flags: review?(etienne)
Attachment #8506085 - Flags: feedback?
Attachment #8506085 - Flags: review?(francisco)
This review fixes a couple of issues that Anshul pointed out in his last feedback from GitHub, and it includes a couple of tests that guarantee that the new flows work as they should.

In my opinion this part of settings requires more tests (after all, we are dealing with pin/puk/fdn flows), but these should be made into a new issue.

I added Francisco as reviewer as well because he is familiar with the bug and the patch.
Because it is a sensitive feature, we need QA to re-test all pin/puk/fdn scenarios from Settings screen. NIing jlorenzo and echang as spoken with jlorenzo on the IRC.
Flags: needinfo?(jlorenzo)
Flags: needinfo?(echang)
Attachment #8506085 - Flags: review?(etienne) → review?(arthur.chen)
(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #35)
> Because it is a sensitive feature, we need QA to re-test all pin/puk/fdn
> scenarios from Settings screen. NIing jlorenzo and echang as spoken with
> jlorenzo on the IRC.

Given this, I am hesitant to land this on 2.1 at this point. The risk would have been manageable before FC as we would have time to fix any fallouts but keeping in mind this has not landed yet or got any bake time on master, I think we should let this get in in 2.2.

Johan, can you give us preliminary assessment of your testing asap, which may help with the decision making here.
No longer blocks: CAF-v2.1-FC-metabug
Comment on attachment 8506085 [details] [review]
Github PR

r+ with the comments on gh fixed.
Attachment #8506085 - Flags: review?(francisco) → review+
Comment on attachment 8506085 [details] [review]
Github PR

EJ, could you help review the patch? Thanks.
Attachment #8506085 - Flags: review?(arthur.chen) → review?(ejchen)
Hi Johan, Do you have any sim card that does not support FDN? I am still trying to find one.
Flags: needinfo?(echang)
The 2 SIM cards I have does support it. I'm looking for one.
Flags: needinfo?(jlorenzo)
Addressed Francisco's comments in the PR
(In reply to bhavana bajaj [:bajaj] from comment #36)
> (In reply to Sergi Mansilla [:sergi] (Telenor) from comment #35)
> > Because it is a sensitive feature, we need QA to re-test all pin/puk/fdn
> > scenarios from Settings screen. NIing jlorenzo and echang as spoken with
> > jlorenzo on the IRC.
> 
> Given this, I am hesitant to land this on 2.1 at this point. The risk would
> have been manageable before FC as we would have time to fix any fallouts but
> keeping in mind this has not landed yet or got any bake time on master, I
> think we should let this get in in 2.2.
> 
> Johan, can you give us preliminary assessment of your testing asap, which
> may help with the decision making here.

I've confirmed this with CAF and given where we are in 2.1 and that its way beyond FL and FC, we will just get this fixed in 2.2 rather than risk the quality so late here. Hence setting the feature-b2g accordingly.
feature-b2g: 2.1 → 2.2+
Comment on attachment 8506085 [details] [review]
Github PR

Left some comments on Github and please check them ! Thanks Sergi.
Attachment #8506085 - Flags: review?(ejchen)
Hi EJ,

I checked your comments and fixed some, and left some comments back. Can you check them?

Thanks!
Flags: needinfo?(ejchen)
Yeah, I left comments back on Github and I think this patch is almost there !

Feel free to set r? on me again, thanks Sergi !
Flags: needinfo?(ejchen)
Blocks: CAF-v2.2-metabug
No longer blocks: CAF-v2.1-CC-metabug
Johan, as we spoke in the standup you can test the non-FDN SIM paths.

Cheers!
Flags: needinfo?(jlorenzo)
No longer blocks: CAF-v2.1-CC-metabug
The patch cannot be imported in master in the current state, some conflicts are reported by Github. Sergi, can you rebase your PR on top of master? I prefer doing the verification after that.
Flags: needinfo?(jlorenzo) → needinfo?(sergi.mansilla)
No longer blocks: CAF-v2.1-CC-metabug
Hi EJ,

Looks like issue Bug 1085313, which you reviewed, is causing the conflicts with this issue, as Johan reports. Parts of Bug 1020757 was refactoring the very code that has been refactored in Bug 1085313, so I'm a bit confused.

Do you think that you or :echen can rebase the this bug (1020757) on top of master now? It is hard for me to know which version is the good one now.
Flags: needinfo?(sergi.mansilla) → needinfo?(ejchen)
No longer blocks: CAF-v2.1-CC-metabug
Sergi, edgar's patch is mainly to remove the use of `event.lockType`. You can check https://github.com/mozilla-b2g/gaia/pull/25371/files#diff-1 for more details.

So for your case, you can use your patch as the base and remember to not to use `lockType` from `event`.

Not sure is this clear to you or not :) ?
Flags: needinfo?(ejchen)
I understand the change, but my patch relies on the event.name to determine what kind of error are we dealing with, but now the event (req.error) is not passed to handleLockError anymore. What to do now? Add a third parameter?
Flags: needinfo?(ejchen)
No longer blocks: CAF-v2.1-CC-metabug
Sergi, I left comment on github and please check it ! I think the better way is to encapsulate the information into an error object and passed it inside the handler. In this way, no matter how interface changed, we can still keep our codes intact with minimum change in the future.

Hope this makes sense to you :)
Flags: needinfo?(ejchen)
No longer blocks: CAF-v2.1-CC-metabug
Sergi, can you please help here ? We are shooting this feature for 2.2 but I want to land this ahead of time given how close we are to wrapping it up and your efforts! Thanks!
Flags: needinfo?(sergi.mansilla)
Yeah I will work on this today.
Flags: needinfo?(sergi.mansilla)
Done. Johan, can you try to test this?
Flags: needinfo?(jlorenzo)
Comment on attachment 8506085 [details] [review]
Github PR

Looks nice to me, r+ with last few nits addressed. Thanks Sergi !
Attachment #8506085 - Flags: review?(ejchen) → review+
Updated the PR with the nits pointed out be EJ.
Flags: needinfo?(jlorenzo)
The patch is ready, but it is sensitive enough that it should be tested by QA. Johan, the contacts QA engineer, doesn't have a SIM that doesn't accept FDN. We are waiting on one, but we don't know when will we get it. So far, the only one with a FDN-less SIM card seems to be Anshul. PErhaps he can help confirming the patch works as expected under a real scenario.
Flags: needinfo?(bbajaj)
Could you update the target milestone? Thank you.
Flags: needinfo?(sergi.mansilla)
Hey Tony,
Mind if you can arrange the testing environment from USA?
Flags: needinfo?(tchung)
(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #57)
> The patch is ready, but it is sensitive enough that it should be tested by
> QA. Johan, the contacts QA engineer, doesn't have a SIM that doesn't accept
> FDN. We are waiting on one, but we don't know when will we get it. So far,
> the only one with a FDN-less SIM card seems to be Anshul. PErhaps he can
> help confirming the patch works as expected under a real scenario.

We have been unable to locate any SIM cards without FDN in the states.   both AT&T and Tmobile are saying they dont support these configurations, and can't get us the hardware.  If anyone has any ideas of has these types of SIMs to test, please take the bug.   Anshul, can you test the patch?
Flags: needinfo?(tchung) → needinfo?(anshulj)
Tony, yes let me spend some time early next week to test it. There was a tip breakage causing boot up issues since last two weeks due to which I couldn't test this change.
Can we update the target milestone? Thank you.
QA Whiteboard: [2.2-feature-qa+]
Flags: in-moztrap?(echang)
We'll need to wait for testing result from Anshul.
Let's say sprint3 which is after xmas vacation.
Target Milestone: 2.1 S7 (24Oct) → 2.2 S3 (9jan)
My test results.

- The original issue of showing puk screen instead of error when a generic error is sent seems to be resolved. I now see a dialog box that says "An unknown error occurred".
- Once the retry count goes 0 and error with the name SimPuk2 is sent to Gaia, I was expecting the SIM puk screen to be displayed but I instead see a SIN PIN2 screen again. Please find the screenshot attached.
- Incorrect password logic seems to be working as expected.
Flags: needinfo?(anshulj)
Flags: needinfo?(bbajaj)
Status: NEW → ASSIGNED
Hi Jenny,

Please check comment 64.
Part-a: I wonder if the general error message is good enough for this case.
Part-b: I would also expect PUK screen. Could you confirm the behavior?
Flags: needinfo?(jelee)
Hello Wesley,

part-a: Are there multiple causes for not being able to activate FDN? I'll have to evaluate the cases and see if one general message is enough. Let me know, thanks!

part-b: Yes, when retry count of SIM PIN 2 is down to 0, show SIM PUK 2.
Flags: needinfo?(jelee)
(In reply to Jenny Lee from comment #67)
> Hello Wesley,
> 
> part-a: Are there multiple causes for not being able to activate FDN? I'll
> have to evaluate the cases and see if one general message is enough. Let me
> know, thanks!
Per discussion with Arthur, Jenny: for now this general message works.
We'll also expect bug 1112471 to land therefore eliminate the case of FDN-less sim card. 

> part-b: Yes, when retry count of SIM PIN 2 is down to 0, show SIM PUK 2.
Not sure if this is an existing bug, or regressed by the patch.
QAWanted to check whether current master runs ok.
Keywords: qawanted
See Also: → 1112471
Need a SIM without FDN support to test, checking prepaid sim in stock.

Hi Johan, Quick check, do you have this kind of card?
Flags: needinfo?(jlorenzo)
Sorry, I still don't have a SIM card without FDN support.
Flags: needinfo?(jlorenzo)
We have related test cases, but without proper sim card for testing.
https://moztrap.mozilla.org/manage/suites/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-name=sim+puk

Hi EJ, Is it possible to test using WebIDE ir simulator?
Flags: needinfo?(ejchen)
Flags: in-moztrap?(echang)
Flags: in-moztrap+
Hi Eric/Johan,

I meant to test this case. Does it still need FDN-less SIM card?

"- Once the retry count goes 0 and error with the name SimPuk2 is sent to Gaia, I was expecting the SIM puk screen to be displayed but I instead see a SIN PIN2 screen again. Please find the screenshot attached."
Eric, if we want to test the full flow like what Anshul said in comment 0, I think we can use emulator to make sure gecko can return "GECKO_ERROR_GENERIC_FAILURE" and then you can see what happened after applying this change (For this part, we can ask Jessica / HsinYi)

While for comment 72, we don't need FDN-less SIM card because we just need to keep pressing wrong passwords to see whether there are two dialogs showing up or not.
Flags: needinfo?(ejchen)
I don't think that Sergi will answer the ni soon, as he has become a father recently.

Can we land this code and then perform a follow up to fix the error in the puk code screen?
Flags: needinfo?(anshulj)
Francisco, if we land this bug in its current state we will break the feature (puk screen not displaying) so I recommend not to land it.
Flags: needinfo?(anshulj)
So, can someone provide an easy way for gaia developers and QA to try out this?
I wonder if we have "Bug 1112484 - [Settings] Show/hide FDN settings based on the availability" landed, do we still need this fix?
Wesley, the proposed patch provides fixes for not just the reported issue but also a bunch of other issues.
(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #72)
> Hi Eric/Johan,
> 
> I meant to test this case. Does it still need FDN-less SIM card?
No it doesn't. In fact the test case below needs a SIM with FDN supported.

> 
> "- Once the retry count goes 0 and error with the name SimPuk2 is sent to
> Gaia, I was expecting the SIM puk screen to be displayed but I instead see a
> SIN PIN2 screen again. Please find the screenshot attached."
So we do need this patch, but we need it w/o the symptom (comment 72). 
Let's defer to next sprint so that maybe Sergi will be available.
For testing, I presume we can count on Anshul for FDN-less case.
As for verifying (comment 72) case, it's easier.
Target Milestone: 2.2 S3 (9jan) → 2.2 S4 (23jan)
Deferring to next sprint as Sergi is on PTO.
Target Milestone: 2.2 S4 (23jan) → 2.2 S5 (6feb)
I can pick it up, but it is mandatory that we have a way to reproduce it that doesn't take weeks of back-and-forth. What is in the way of getting a FDN-less card? If there is no way we can get one, perhaps we can get some help to determine what's the exact error flow that a FDN-less card would give us.
Flags: needinfo?(whuang)
Flags: needinfo?(sergi.mansilla)
Flags: needinfo?(francisco)
Wesley, this has been a really difficult task since we don't have the hardware to test it. If parnters want this bug solved, they must provide a way for us to test it, and reproduce it correctly.
Flags: needinfo?(francisco)
Sergi and Francisco, as per comment #79 the regression with this patch is with a SIM that requires FDN. Can you guys not hack gecko in your local build to simply return failure for enable/disable FDN for the rest of the test cases?

I am okay with testing your proposes patch again if needed.
Hi Francisco and Sergi,
I had asked Josh to figure out longer term plan for FDN-less testing, as he is our contact to outsourcing QA. They have some plans but I don't feel it's going to be quick enough for this bug and 2.2FL.

At this point, the most practical method now is to rely on Anshul for FDN-less cases testing.
On the other hand, the current issue (per comment 64, bullet#2) actually requires FDN sims, so I suppose it's not a bottleneck. Any comment?
Flags: needinfo?(whuang)
Hey,
I suppose we're not blocked. 
Can we proceed?
Flags: needinfo?(sergi.mansilla)
Francisco & Sergi,

Can either of you reply here with an update on what the path forward is? It's currently unclear for the past week and CAF (anshul) is ready to test whatever changes you provide.

Thanks,
Mike
Flags: needinfo?(francisco)
Sergi will you have time to take a look to this?
Flags: needinfo?(francisco)
Through email he mentioned that he'll put time on it this week.
Hi Sergi,
Please let us know how it goes.
I can help this bug.
Assignee: sergi.mansilla → ejchen
Component: Gaia::Contacts → Gaia::Settings
Ahhh Anshul,

when I reviewed this bug, Gecko didn't provide the API to let us know whether the simcard has the availability of FDN, so we need to handle such case in Gaia.

But with bug 1112471 (gecko patch) + bug 1112484 (gaia patch) (They were all landed in v2.2 on Jan, 2015), right now we will show/hide the menuItem based on the availability of FDN, so based on your comment 0, there should be no chances for users to get inside that panel and this bug would be invalid.

Can you use latest Gaia to verify my comment ? If there is anything missing from my point above, please correct me to let me know, thanks :)
Flags: needinfo?(anshulj)
Sorry Anshul, I missed comment 78.

If our core bug has been fixed base on those two bugs, can you update the bug description + title or list down what other issues would be fixed ? By doing so, this can make us more clear on what we are going to do next. 

Thanks !!
No longer blocks: CAF-v3.0-FL-metabug
Ej Chen, I agree with your comment #93. However you have some good fixes for other issues as well in your PR so we should still get those in; perhaps as a separate bug. That way this bug is not blocking our release anymore.
Flags: needinfo?(anshulj)
Anshul, because simcard related scripts are updated a lot in master, so I made two different patches.

When writing v2.2 patch, I noticed that in our latest v2.2 codebase, we can't click the '<' button to leave the page when users try to enter pin codes. Not sure whether this syndrome happens there too ? 

Can you help me check these two patches to see whether them fixed problems or not. Thanks !!
Flags: needinfo?(anshulj)
Contacted Anshul & Inder (CAF PoC) by email today requesting a response to comment 97. Awaiting CAF's response:


From: Michael Lee <mlee@mozilla.com>
Subject: Your help is needed with Bug 1029757
Date: March 18, 2015 at 11:25:04 AM PDT
To: anshulj@codeaurora.org
Cc: "Kumar, Indrajeet" <ikumar@quicinc.com>

Hi Anshul,

Please respond to Bug 1020757’s comment 97 [1]. The assigned engineer needs your help to test the behavior of two patches he’s created:

<comment 97's text>

Thanks,
Mike

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1020757#c97
(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #97)
> Anshul, because simcard related scripts are updated a lot in master, so I
> made two different patches.
> 
> When writing v2.2 patch, I noticed that in our latest v2.2 codebase, we
> can't click the '<' button to leave the page when users try to enter pin
> codes. Not sure whether this syndrome happens there too ? 
> 
> Can you help me check these two patches to see whether them fixed problems
> or not. Thanks !!

EJ, tested your patch on 2.2 and ther original issue reported is fixed. Your patch also fixes the regression I mentioned in comment #64. I however see some issues while updating FDN contact. I am not sure if they are related to your patch, most likey not, so for now I have filed a separate bug for the new issue at bug 1145332.
Flags: needinfo?(anshulj)
Attachment #8576554 - Flags: feedback+
Comment on attachment 8576554 [details] [review]
[gaia] EragonJ:v2.2-bug-1020757 > mozilla-b2g:v2.2

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

Arthur, can you help me review this patch ? Thanks :)
Attachment #8576555 - Flags: review?(arthur.chen)
Thanks Anshul, for this bug, let's focus on fixing the problem about handling the generic error case. While for the problem you mentioned at bug 1145332, I'll check it later and report there if I found any clues.
Flags: needinfo?(sergi.mansilla)
Attachment #8576554 - Flags: review?(arthur.chen) → review+
Comment on attachment 8576555 [details] [review]
[gaia] EragonJ:bug-1020757 > mozilla-b2g:master

I noticed that the input is not cleared when retrying. Not sure if it was regressed by the refactoring or due to this patch. Could you help check it? Thanks.
Attachment #8576555 - Flags: review?(arthur.chen)
Comment on attachment 8576555 [details] [review]
[gaia] EragonJ:bug-1020757 > mozilla-b2g:master

Thanks Arthur, the problem you mentioned may be a small regression when refactoring this panel in Master. In order to make UI looks nice, I only reset the input value back to empty when some errors happen.

Do you think this patch makes sense to you :) ?
Attachment #8576555 - Flags: review?(arthur.chen)
For this 2.2 patch, we need one more l10n string for generic error, so add late-l10n flag here.
Keywords: late-l10n
Comment on attachment 8576555 [details] [review]
[gaia] EragonJ:bug-1020757 > mozilla-b2g:master

r=me with the comment addressed. We should also clear the input when lockType is puk2. And both the new pin input and confirm input need to be cleared. Thanks.
Attachment #8576555 - Flags: review?(arthur.chen) → review+
Thanks Arthur, in addition to the comments you left, I also improved the other parts like changeSimPin ... etc.

For now, I think SimPinDialog works and looks better ! :)
Looks like our try-server-bot is taking some days off (!?), so I will land this patch soon after it is coming back :)
Thanks all, the patch was merged at Gaia/master : https://github.com/mozilla-b2g/gaia/commit/6f38eecfb11ff1b81771e76696fdafcdfdadc405
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8576554 [details] [review]
[gaia] EragonJ:v2.2-bug-1020757 > mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no
[User impact] if declined: We can't handle some generic error case correctly.
[Testing completed]: yes, partner helped us to verify this patch already.
[Risk to taking this patch] (and alternatives if risky): low 
[String changes made]: yes, I added one more string to hint users about error, so I added late-l10n flag here.
Attachment #8576554 - Flags: approval-gaia-v2.2?
Attachment #8576554 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.