916059 - [FDN] broken workflow when a wrong PIN2 code is used

Status: RESOLVED FIXED

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

Description

[Fabien Cazenave [:kaze]]

STR:
 • Settings > Call Settings > Fixed dialing numbers
 • make sure FDN is enabled
 • Authorized numbers
 • add a new FDN contact, tap [OK]
 • enter a wrong PIN2

Expected:
 • be prompted to enter a wrong PIN2 again

Actual:
 • UI gets back to the FDN contact list

Comment 1

[Ivan Tsay (:ITsay)]

Hi Joe,

Could you add keyword in whiteboard for your team to track on this?

Comment 2

[Alex Keybl [:akeybl]]

Any updates on where this is at, and when it will be complete? This blocks QC CS (which we're trying to pull in as much as possible), but hasn't had comment from you in a month.

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

Michal, is this bug something you can look into? Thanks

Comment 4

[Michael Vines [:m1] [:evilmachines]]

Joe, this bug seem to have zero traction at the moment.  Please help get this bug moving.

Comment 5

[Michał Budzyński (:michalbe)]

I'm running workshop for partners till the end of next week, but :kaze is back form his PTO.

Comment 6

[Fabien Cazenave [:kaze]]

Finishing the Gaia part atm. I’ve filed bug 936309 and bug 936334 for the Gecko part.

Comment 7

[Fabien Cazenave [:kaze]]

Attachment: link to pull request

Comment 8

[Fabien Cazenave [:kaze]]

Comment on attachment 830829 [details] [review]
link to pull request

Just found some new issues — clearing the review flag until I fix them.

Comment 9

[Fabien Cazenave [:kaze]]

Comment on attachment 830829 [details] [review]
link to pull request

Should be OK now.

Comment 10

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

Comment on attachment 830829 [details] [review]
link to pull request

Please check my comments in github, thanks!

Comment 11

[Fabien Cazenave [:kaze]]

Comment on attachment 830829 [details] [review]
link to pull request

Comments addressed.

Comment 12

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

Comment on attachment 830829 [details] [review]
link to pull request

r=me with one nit addressed. Thanks!

Comment 13

[Fabien Cazenave [:kaze]]

Master:   https://github.com/mozilla-b2g/gaia/commit/ac42cb33f21b3f13595432c965f44615daae2225
v1.2/koi: https://github.com/mozilla-b2g/gaia/commit/1fb5d3168c95201d90bdb8f3c3d9023f77fffbed

Comment 14

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Fabien,

This patch will make user unable to change SIM PIN in Settings. 
Steps:
1. Settings -> SIM security -> change PIN
Expected:
  Display SIM PIN change dialog
Actual: 
  Stay at SIM security page

From logcat:
E/GeckoConsole( 2529): [JavaScript Error: "TypeError: options is undefined" {file: "app://settings.gaiamobile.org/js/simcard_dialog.js" line: 410}]

I guess this is because of the signature change of SimPinDialog.show() (see https://github.com/mozilla-b2g/gaia/commit/d12bf7adc72f52196927d0f3babb6fa93fffc585#diff-a888a0705d0739307eaed9b0bef204bdR398), and all the invocations of show() at apps/settings/js/simcard_lock.js did not aware of this change.

One example, at https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/simcard_lock.js#L91, the call to show() did not provide second argument. Thus, when SimPinDialog.show tries to access 'options', the only thing it can get is undefined. That's why line 410 failed.

This patch is quite new, so I think it would be better to notify you here instead of open new bug.

Thanks

Comment 15

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Filed bug 940162 for it, clear flag

Comment 16

[Jason Smith [:jsmith]]

Note - there's quite a lot of fallout being seen with this patch. Are we sure we shouldn't back this out & retry landing this with necessary fixes?

Comment 17

[Preeti Raghunath(:Preeti)]

Kaze,

This patch is causing at least 2 blocker regressions. Given where we are with the release, I'd like to see what it would take to back out this bug.

Comment 18

[Fabien Cazenave [:kaze]]

Preeti, these regression should be described and fixed by bug 940162 (which has already been uplifted to 1.2).

If there are other regressions that aren’t fixed by this patch, please let me know (bug#?) and I’ll back-out these two patches (916059 + 940162).