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

Status: RESOLVED FIXED

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

Description

[Anshul]

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.

Comment 1

[Anshul]

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.

Comment 2

[Anshul]

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

Comment 3

[Jason Smith [:jsmith]]

Does this happen on 1.4?

Comment 4

[Anshul]

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

Comment 5

[Jason Smith [:jsmith]]

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.

Comment 6

[bhavana bajaj [:bajaj]]

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 ?

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

This is a comms team feature.

Comment 8

[David Scravaglieri [:scravag]]

Sergi, could you take a look at this please ?

Comment 9

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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?

Comment 10

[Anshul]

Sergi, you would have to hack your code to reproduce this scenario. If you upload your patch, I can give it a shot.

Comment 11

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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!

Comment 12

[Anshul]

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.

Comment 13

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Anshul,

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

Thanks!

Comment 14

[Anshul]

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?

Comment 15

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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!

Comment 16

[Anshul]

Sorry for the delay Sergi; busy with commercialization of 2.0. Will get back to you in a day or two.

Comment 17

[Anshul]

Sergi, I left some comments for you on github.

Comment 18

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Anshul, left some comments in the Github PR. Getting there!

Comment 19

[Anshul]

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.

Comment 20

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Needinfo to notify the ETA changes.(see whiteboard)
Please be noted that also depends on the cadence of testing feedback.

Comment 21

[Anshul]

Thanks Wesley.

Comment 22

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 23

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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

Comment 24

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Right now, the assumption that FDN is disabled given certain conditions happens here: https://github.com/mozilla-b2g/gaia/pull/24299/files#diff-a888a0705d0739307eaed9b0bef204bdR152

Comment 25

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Merged at e1fd99454b6cd5da4f2c58f928fc04c6d03f478f

Comment 26

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #25)
> Merged at e1fd99454b6cd5da4f2c58f928fc04c6d03f478f

Please disregard that comment. It concerns another bug.

Comment 27

[Anshul]

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

Comment 28

[bhavana bajaj [:bajaj]]

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?

Comment 29

[Anshul]

Bhavana, this is not critical for 2.1.

Comment 30

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

I expect to have a candidate fix at the end of today, CET.

Comment 31

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

The fix is there, but unfortunately this needs a lot of testing. I will need more time to write unit tests, 2-3 days.

Comment 32

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

The new pull request is here: https://github.com/mozilla-b2g/gaia/pull/25193

Comment 33

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR

Comment 34

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 35

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 36

[bhavana bajaj [:bajaj]]

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

Comment 37

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8506085 [details] [review]
Github PR

r+ with the comments on gh fixed.

Comment 38

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8506085 [details] [review]
Github PR

EJ, could you help review the patch? Thanks.

Comment 39

[Eric Chang [:ericcc] [:echang]]

Hi Johan, Do you have any sim card that does not support FDN? I am still trying to find one.

Comment 40

[Johan Lorenzo [:jlorenzo]]

The 2 SIM cards I have does support it. I'm looking for one.

Comment 41

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Addressed Francisco's comments in the PR

Comment 42

[bhavana bajaj [:bajaj]]

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

Comment 43

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8506085 [details] [review]
Github PR

Left some comments on Github and please check them ! Thanks Sergi.

Comment 44

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi EJ,

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

Thanks!

Comment 45

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 !

Comment 46

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Johan, as we spoke in the standup you can test the non-FDN SIM paths.

Cheers!

Comment 47

[Johan Lorenzo [:jlorenzo]]

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.

Comment 48

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 49

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :) ?

Comment 50

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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?

Comment 51

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :)

Comment 52

[bhavana bajaj [:bajaj]]

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!

Comment 53

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Yeah I will work on this today.

Comment 54

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Done. Johan, can you try to test this?

Comment 55

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8506085 [details] [review]
Github PR

Looks nice to me, r+ with last few nits addressed. Thanks Sergi !

Comment 56

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Updated the PR with the nits pointed out be EJ.

Comment 57

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 58

[Kai-Chih Hu [:khu]]

Could you update the target milestone? Thank you.

Comment 59

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hey Tony,
Mind if you can arrange the testing environment from USA?

Comment 60

[Tony Chung [: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?

Comment 61

[Anshul]

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.

Comment 62

[Kai-Chih Hu [:khu]]

Can we update the target milestone? Thank you.

Comment 63

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

We'll need to wait for testing result from Anshul.
Let's say sprint3 which is after xmas vacation.

Comment 64

[Anshul]

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.

Comment 65

[Anshul]

Attachment: enable_fdn_instead_of_sim_puk2

Comment 66

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

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?

Comment 67

[Jenny Lee]

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.

Comment 68

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

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

Comment 69

[Eric Chang [:ericcc] [:echang]]

Need a SIM without FDN support to test, checking prepaid sim in stock.

Hi Johan, Quick check, do you have this kind of card?

Comment 70

[Johan Lorenzo [:jlorenzo]]

Sorry, I still don't have a SIM card without FDN support.

Comment 71

[Eric Chang [:ericcc] [:echang]]

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?

Comment 72

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

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

Comment 73

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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.

Comment 74

[Francisco Jordano [:arcturus] [:francisco]]

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?

Comment 75

[Anshul]

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.

Comment 76

[Francisco Jordano [:arcturus] [:francisco]]

So, can someone provide an easy way for gaia developers and QA to try out this?

Comment 77

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

I wonder if we have "Bug 1112484 - [Settings] Show/hide FDN settings based on the availability" landed, do we still need this fix?

Comment 78

[Anshul]

Wesley, the proposed patch provides fixes for not just the reported issue but also a bunch of other issues.

Comment 79

[Anshul]

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

Comment 80

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

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.

Comment 81

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Deferring to next sprint as Sergi is on PTO.

Comment 82

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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.

Comment 83

[Francisco Jordano [:arcturus] [: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.

Comment 84

[Anshul]

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.

Comment 85

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

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?

Comment 86

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hey,
I suppose we're not blocked. 
Can we proceed?

Comment 87

[Mike Lee [:mlee]]

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

Comment 88

[Francisco Jordano [:arcturus] [:francisco]]

Sergi will you have time to take a look to this?

Comment 89

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Through email he mentioned that he'll put time on it this week.

Comment 90

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Sergi,
Please let us know how it goes.

Comment 91

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

I can help this bug.

Comment 92

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :)

Comment 93

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 !!

Comment 94

[Anshul]

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.

Comment 95

[GitHub Autolander Bot]

Attachment: [gaia] EragonJ:v2.2-bug-1020757 > mozilla-b2g:v2.2

Comment 96

[GitHub Autolander Bot]

Attachment: [gaia] EragonJ:bug-1020757 > mozilla-b2g:master

Comment 97

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 !!

Comment 98

[Mike Lee [:mlee]]

Contacted Anshul & Inder (CAF PoC) by email today requesting a response to comment 97. Awaiting CAF's response:


From: Michael Lee <[e-mail]>
Subject: Your help is needed with Bug 1029757
Date: March 18, 2015 at 11:25:04 AM PDT
To: [e-mail]
Cc: "Kumar, Indrajeet" <[e-mail]>

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

Comment 99

[Anshul]

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

Comment 100

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :)

Comment 101

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8576555 [details] [review]
[gaia] EragonJ:bug-1020757 > mozilla-b2g:master

Arthur, can you help me review this patch ? Thanks :)

Comment 102

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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.

Comment 103

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.

Comment 104

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 :) ?

Comment 105

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

For this 2.2 patch, we need one more l10n string for generic error, so add late-l10n flag here.

Comment 106

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.

Comment 107

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 ! :)

Comment 108

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Looks like our try-server-bot is taking some days off (!?), so I will land this patch soon after it is coming back :)

Comment 109

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks all, the patch was merged at Gaia/master : https://github.com/mozilla-b2g/gaia/commit/6f38eecfb11ff1b81771e76696fdafcdfdadc405

Comment 110

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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.

Comment 111

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/0ca5b0e59d19e2e260f6d7b1f77374e6dfc60c52