1017224 - [B2G][Settings]Firefox Account Sign-in: Verification message does not display until killing and relaunching the settings app

Status: VERIFIED FIXED

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

Description

[Andrew Stole [:astole]]

After creating a new Firefox Account and verifying it, it isn't possible to completely log into the account. Settings has to be fully closed in order to proceed to the account page and get the verification message.

Repro Steps:
1) Update a Flame to BuildID: 20140528040205
2) Open Settings app and scroll down and tap on 'Firefox Accounts'
3) Enter an email address that has not been verified
4) Enter age over 13
5) Enter password and press Done
6) Press the 'X' to return to the Settings app
7) Verify account through email sent to the email address from step 3

Actual:
Verify message does not display until the Settings app has been killed and relaunched

Expected:
Settings app does not have to be force closed in order to receive the verification message

2.0 Environmental Variables:
Device: Flame 2.0 MOZ
BuildID: 20140528040205
Gaia: bc6f07c149770c6e6dfbea941ac65138dc364a15
Gecko: e017c15325ae
Version: 32.0a1
Firmware Version: v10G-2

Repro frequency: 100%
See attached: Video, logcat

Comment 1

[Andrew Stole [:astole]]

User agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Issue DOES occur on 2.0 Buri. Verification message does not display until the Settings app has been killed and relaunched

Environmental Variables:
Device: Buri v2.0 Master M-C Mozilla RIL
BuildID: 20140528040205
Gaia: bc6f07c149770c6e6dfbea941ac65138dc364a15
Gecko: e017c15325ae
Version: 32.0a1
Firmware Version: v1.2-device.cfg

This option is NOT available on 1.4.

Comment 2

[Andrew Stole [:astole]]

Attaching video of tapping on Firefox Accounts option in Settings app before and after force closing Settings.

Comment 3

[Andrew Stole [:astole]]

Attachment: logcat

Attaching logcat

Comment 4

[Jason Smith [:jsmith]]

Seems like this is something basic not working here.

Comment 5

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

Verified. This is really unfortunate. Digging in to see if it's Gecko not sending the callbacks, or Gaia not receiving them.

Comment 6

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

Still digging, but I am pretty sure the problem is this:

The settings app gets fxa events ('login', 'logout') from gecko through /shared/js/fxa_iac_client. This helper is written as though it were a singleton: it registers/deregisters listeners, and pushes fxa events to each listener as it receives them from gecko.

So, what happens if >1 app instantiates an fxa_iac_client? Does gecko manage multiple listeners for these events? Does it simply overwrite the pointer to the most recent subscriber? Does it ignore all but the first subscriber? What happens if an app (like Settings) is minimized, then reopened?

Continuing to dig.

Comment 7

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

Update: I found one bug, not sure it's *the* bug.

Whenever an IAC helper instance calls getAccounts() for the first time, the 'verified' key is never present in the response. All we get is the email.

The Settings UI code expects the 'verified' key is *always* present, and only displays the verified logged-in state if event.verified is present and true.

Reproing the getAccounts error after signup, before verifying:

1. open fxa test client, logged out state
2. sign up for a new account
3. after dialog is dismissed, click 'get accounts', and you will see a JSON blob with no 'verified' field
4. click 'get accounts' again, and you *will* see the 'verified' field is present, and its value is 'false'
5. click the verify link in your browser, wait a few seconds
6. click 'get accounts' again, the 'verified' field is always present

Reproing the getAccounts error after signing up and verifying:

1. test client, click 'log out'
2. test client, open flow and sign up
3. click verify link in a browser
4. now, click 'get accounts'
5. the JSON packet only has an 'email' field
6. click 'get accounts' again
7. 'verified':'true' is now present

To be continued tomorrow.

Comment 8

[Sam Penrose]

Jared, I landed a couple of Gecko patches over the weekend that may have affected this bug. I am taking a look at exactly how the events flow in Gecko, which should help narrow down the problem.

Comment 9

[Sam Penrose]

With the latest mozilla-central, verifying the email causes Gecko to lob an event towards the system app:

  mozFxAccountsUnsolChromeEvent: fxaccounts:onverified

The last lines of fxa_manager.js are listening for:

  onverifiedlogin

when I edit them to catch the actual string, Gecko then immediately receives a getAccounts() call, so yay progress. The call returns a mozFxAccountsChromeEvent with {"email":"[e-mail]","verified":true}, but the Settings UI doesn't refresh. Happy to dig further; LMK.

Comment 10

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

Hey Sam - Sounds like you're already looking into this, so feel free to steal it if your schedule allows.

Comment 11

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

This has been sitting for a few days, stealing it back. Shout if you've got code ready to go.

Comment 12

[Sam Penrose]

Attachment: 1017224-msg-string.patch

Jared I will totally let you steal this back. Here's a bit to smooth the message passing; you can of course unify the Gaia spelling to match Gecko.

Comment 13

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

Awesome! thanks.

Comment 14

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

Attachment: Github PR 20517

Comment 15

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

Comment on attachment 8440188 [details] [review]
Github PR 20517

Hi again, gents!

Here's another straightforward b2g-blocking bug fix, please take a look when you have a moment.

Thanks!

Jared

Comment 16

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8440188 [details] [review]
Github PR 20517

r=me for the non-Settings part if you do the s/onverifiedlogin/onverified also on:

https://mxr.mozilla.org/gaia/source/dev_apps/test-fxa-client/js/app.js#53
https://mxr.mozilla.org/gaia/source/dev_apps/test-fxa-client/js/app.js#54
https://mxr.mozilla.org/gaia/source/shared/js/fxa_iac_client.js#226

Comment 17

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

It seems the bug was due to that the event name was changed from "onverifiedlogin" to "onverified"?

As my email addresses were all registered to firefox accounts, is there any other way to test it (or ways that I can use to revoke my account)?

Comment 18

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

Hi Arthur,

The way I generally test signups is to use an existing email address, like [e-mail], but append '+' and a random string to the end of the user part, like [e-mail]. The email will still be delivered, but it will look like a new account to firefox accounts, so you can go through signup, click the emailed verification link, and check that onverified behavior works properly.

Comment 20

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

Is the expected behavior like once I've verified the account, the item will then change to signed in as [e-mail]? If that's the case, the patch does not fix the issue. I still need to relaunch the settings app to see the signed in message.

Comment 21

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

(In reply to Arthur Chen [:arthurcc] from comment #20)
> Is the expected behavior like once I've verified the account, the item will
> then change to signed in as [e-mail]? If that's the case, the patch does
> not fix the issue. I still need to relaunch the settings app to see the
> signed in message.

Yup, that's the expected behavior: after verification, the settings app automatically updated from unverified to verified screens.

I will try to repro on my Flame today and look for lingering bugs :-(

Comment 22

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

Hey Arthur,

The reported bug seems fixed to me, but I found (and fixed) a different bug when trying to reproduce your problem :-)

Two cases that work reliably for me:

Panel auto-updating:
1. go to settings fxa panel, sign up, leave the fxa panel visible
2. click verification email link in a desktop email client (settings app stays visible)
3. fxa panel updates correctly
4. after closing out of the panel, the fxa menu item also is updated

Menu auto-updating:
1. go to settings fxa panel, sign up, exit the fxa panel, so that fxa menu item is visible.
2. click verification email link in a desktop email client
3. fxa menu item updates correctly
4. click into fxa panel, it is also updated

I always use a second device to click the verify link when manually testing, it's just quickest.

I realized that, if the settings app is not visible when the verify link is clicked, then the UI is incorrect:
1. sign up from settings
2. close the settings app, go to homescreen
3. click verification link in a desktop email client
4. reopen the settings app
5. the settings app shows the wrong state!

This is happening because we detach/re-attach the event listeners on visibility change, but don't refresh the UI state after the app is shown again.

I've fixed this and pushed the update. Would you mind having another look?

Comment 23

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

When I tried to create a new account I was stuck after entering the password. It kept displaying the busy icon and displayed "connecting". Note that I still received the confirm mail and was able to verify it. The account was created successfully.

Gaia      b4c7cc2de31a2d73bca75467cfd11cc3479ac2c0
Gecko     https://hg.mozilla.org/mozilla-central/rev/f78e532e8a10
BuildID   20140618160204
Version   33.0a1

Comment 24

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

Hey Arthur,

That's odd. There is a known bug (bug 998464) around setting a timer to dismiss the 'Connecting...' overlay, but it only seems to happen when you lose network connectivity *after* starting the signup flow. Do you think that might be what happened in your case?

No-Jun, would you mind trying to repro this bug today? I'll try as well.

Comment 25

[No-Jun Park [:njpark]]

(In reply to Jared Hirsch [:_6a68] [@6a68] from comment #24)
> Hey Arthur,
> 
> That's odd. There is a known bug (bug 998464) around setting a timer to
> dismiss the 'Connecting...' overlay, but it only seems to happen when you
> lose network connectivity *after* starting the signup flow. Do you think
> that might be what happened in your case?
> 
> No-Jun, would you mind trying to repro this bug today? I'll try as well.

This is not reproducible in 2.0, but looks like there is a gecko bug causing the issue:
Bug 1027874

Comment 26

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

Hi Arthur,

Would you mind taking another shot at verifying this fix?

Gecko bug 1027874 has a fix, but hasn't landed in m-c yet. For now, the pvt build from the previous day (20140618040513) works correctly for me.

Thanks!

Jared

Comment 27

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

Comment on attachment 8440188 [details] [review]
Github PR 20517

I think the patch is good but it only works on v2.0 but not on master, and that's why I'm not able to give r+ to this patch for now. I would prefer to test the patch with bug 1027874 landed in master. Could you request a review again when ready? Thank you!

Comment 28

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

Comment on attachment 8440188 [details] [review]
Github PR 20517

Hi Arthur,

The gecko bug 1027595 has been fixed, and this patch looks good to me when applied to today's build.

Would you mind taking another look?

Thanks,

Jared

Comment 29

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

Comment on attachment 8440188 [details] [review]
Github PR 20517

Verified ok with the latest pvt build and the provided patch. Please squash the commits before merging, thanks!

Comment 30

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

Thanks Arthur!

Rebased and pushed, just waiting for Gaia to reopen[1] to merge this.

[1] https://groups.google.com/d/topic/mozilla.dev.gaia/taBuowY7MbE/discussion

Comment 31

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

Travis is green, except for a known marionette bug:

https://travis-ci.org/mozilla-b2g/gaia/builds/28361779

Merging to master.

Comment 32

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

doh, didn't actually click the green button earlier.

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

Comment 33

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/8615a1f31e73d365c1aa9ba74477245d28de6ef3

Comment 34

[Alissa（leave from Mozilla）]

Attachment: verify.3gp

This issue has been successfully verified on Flame 2.1, 2.0
See attachment:verify.3gp
Reproducing rate: 0/5

flame2.1 new build:
Gaia-Rev        5655269098c7e82254e56933f1af05b4abe2a2f3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/86608c9389b5
Build-ID        20141204001201
Version         34.0

Flame 2.0 versions:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/ff1100ba2ab8
Build-ID        20141204000228

Version         32.0