980638 - FTU 'forgot password' link should show an error message, not enter fxa web reset flow

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FxA, defect, P2)

Comment 1

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Actually the problem here seems to be that FtuLauncher.isFtuRunning() *always* returns false.

So, I'll have to pass the name of the caller into the openFlow() call, and we'll have to set some state inside the system app.

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

ferjm does this seem right? or should I fix FtuLauncher? (see my previous comment)

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

It sounds like a bug in FtuLauncher, so I would prefer to fix it instead of doing the workaround in the fxa code.

Comment 5

[Jason Smith [:jsmith]]

See the dupe for impact.

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Yup, filing under our 2.0 gaia blockers bug tree :-)

Comment 7

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

The COPPA error message also contains a link which should not be displayed if we are inside FTE, since, again, the user will get stuck. Related to bug 994911.

Comment 8

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Doh, now I see the problem.

The fxa system app runs in its own iframe, so it doesn't have access to the same web storage as the rest of the FTU app.

I missed a crucial bit of code[1] in which the fxa panel in the FTU app puts a specific class on the system app dialog ('isFTU') at system app startup time, we can use this from the system app to figure out if we're inside FTU or not.

Still working out another bug in the reset password flow, will submit when it's really working in both cases.

[1] https://github.com/mozilla-b2g/gaia/blob/ee67268/apps/system/js/fxa_ui.js#L79-L83

Comment 9

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Enter password page looks good, just need to update COPPA page and we're done.

Comment 10

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Hi Matej,

I've needed to modify the COPPA error message very slightly for FTU. The spec[1] (screen 19.4) uses this error message if the age verification fails:

"You must meet certain age requirements to use a firefox account. Learn more."

(where "Learn more" is a link that goes off to an FTC site about COPPA.)

If the user fails the COPPA check during FTU, I think we do not want to send them off to some other website--we want them to finish FTU. So, I've modified the COPPA failure message displayed during FTU to read:

"You must meet certain age requirements to use a firefox account. Try again later from Settings."

It's not the most lucid, thrilling, brilliant prose ever authored, but it seems to cover the gist. Does this modification seem OK?

Comment 11

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Github PR 18657

Comment 12

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8412347 [details] [review]
Github PR 18657

Hi Fernando,

Mind reviewing this? Should be pretty quick--the problem wasn't FtuLauncher, I was checking FTU status the wrong way.

Travis is green.

Thanks!

Comment 13

[Matej Novak [:matej]]

Hi Jared,

The copy looks good to me. I just want to make sure that "Firefox Account" is capitalized in the string.

Thanks.

Comment 14

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Awesome, thanks Matej. There's a separate bug with the final copy (bug 987418), I'll make sure "Firefox Account" is capitalized once that lands (assuming this bug lands first).

Comment 15

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8412347 [details] [review]
Github PR 18657

LGTM. Thanks Jared.

Comment 16

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Master: https://github.com/mozilla-b2g/gaia/commit/9165b9c503825619fba9afec5dfd43f98a7e40ff

Comment 17

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Reopening, njpark discovered this is not working on nightly.

I see this error in logcat when I hit the forgot link:

E/GeckoConsole(  136): [JavaScript Error: "TypeError: this.fxaDialog is null" {file: "app://system.gaiamobile.org/fxa/js/screens/fxam_enter_password.js" line: 55}]

Comment 18

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Thanks for setting that flag, Jason :-)

This has been held up by bug 1001348, which is now fixed, so we'll be able to debug certified app code again. Hoping to get back to it today.

Comment 19

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Github PR 19088

Hey Fernando,

I'm not quite sure how this was working before, but now it is *really* working :-P

Mind having a look?

Thanks!

Jared

Comment 20

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Missed a spot, and travis is cranky. Unsetting r? for a moment

Comment 22

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Hi Fernando,

Do you have a few for a simple review?

The original problem was detecting if FTU is running, and having the FxA system app do something special if 'reset password' was tapped. I wasn't detecting FTU state properly, because the FtuLauncher inside the FxA iframe didn't have access to the FtuLauncher web storage from the top frame.

I've fixed FTU detection by having the parent context set window.name='isFTU' on the FxA iframe, if FTU is running.

Since the COPPA screen also shows two different error messages, depending on whether FTU is running, I've fixed that here as well.

Tests have also been updated, and Travis is green.

Thanks!

Jared

Comment 23

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Whoops, left a console.log in there. Unsetting r? for the moment.

Comment 24

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Hey Fernando - I think this is finally really ready. Do you mind having a look? Thanks!

Comment 25

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Thanks Jared.

The approach you are taking in this patch seems to work fine, but I'd like to try a different approach as we already have a mechanism for passing information to the FxA iframe when opening the flow from FxAccountsUI.

Instead of overloading the 'window.name' property, I was wondering if it would be possible to add a 'isftu' or something query parameter at [1]. These parameters are set at [2] and you can later access to its values from any of the FxA screens.

Thanks!

[1] https://github.com/mozilla-b2g/gaia/pull/19088/files#diff-a22a03859e26d93d0df8a9ad28dead2fL74
[2] https://github.com/6a68/gaia/blob/bug-980638-resubmitted/apps/system/fxa/js/fxam_manager.js#L30

Comment 26

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Hey Fernando,

Thanks for pointing out the query params approach. I've updated the patch to use that instead of window.name. Tests are green. Do you have time to review the updated patch? The latest changes are in a separate commit.

Cheers,

Jared

Comment 27

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8419837 [details] [review]
Github PR 19088

Thanks Jared. This way looks better :)

Comment 28

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Awesome! Thanks Fernando :-)

Comment 29

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Master: https://github.com/mozilla-b2g/gaia/commit/b3069aef41628fd740da1a23846d408df61c5014