Closed
Bug 1017224
Opened 10 years ago
Closed 10 years ago
[B2G][Settings]Firefox Account Sign-in: Verification message does not display until killing and relaunching the settings app
Categories
(Firefox OS Graveyard :: FxA, defect, P2)
Tracking
(blocking-b2g:2.0+, b2g-v1.4 unaffected, b2g-v2.0 verified, b2g-v2.1 verified)
Tracking | Status | |
---|---|---|
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | verified |
b2g-v2.1 | --- | verified |
People
(Reporter: astole, Assigned: jhirsch)
References
()
Details
(Whiteboard: [fxa4fxos2.0])
Attachments
(4 files)
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
Reporter | ||
Comment 1•10 years ago
|
||
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.
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → affected
Reporter | ||
Comment 2•10 years ago
|
||
Attaching video of tapping on Firefox Accounts option in Settings app before and after force closing Settings.
Reporter | ||
Comment 3•10 years ago
|
||
Attaching logcat
Comment 4•10 years ago
|
||
Seems like this is something basic not working here.
blocking-b2g: --- → 2.0?
Component: Gaia::Settings → FxA
Assignee | ||
Comment 5•10 years ago
|
||
Verified. This is really unfortunate. Digging in to see if it's Gecko not sending the callbacks, or Gaia not receiving them.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → 6a68
Priority: -- → P2
Updated•10 years ago
|
Whiteboard: [fxa4fxos2.0]
Assignee | ||
Comment 6•10 years ago
|
||
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.
Assignee | ||
Comment 7•10 years ago
|
||
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.
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 8•10 years ago
|
||
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•10 years ago
|
||
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":"10172243@mailinator.com","verified":true}, but the Settings UI doesn't refresh. Happy to dig further; LMK.
Assignee | ||
Comment 10•10 years ago
|
||
Hey Sam - Sounds like you're already looking into this, so feel free to steal it if your schedule allows.
Updated•10 years ago
|
Assignee: 6a68 → spenrose
Assignee | ||
Comment 11•10 years ago
|
||
This has been sitting for a few days, stealing it back. Shout if you've got code ready to go.
Assignee: spenrose → 6a68
Comment 12•10 years ago
|
||
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.
Assignee | ||
Comment 13•10 years ago
|
||
Awesome! thanks.
Assignee | ||
Comment 14•10 years ago
|
||
Assignee | ||
Comment 15•10 years ago
|
||
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
Attachment #8440188 -
Flags: review?(ferjmoreno)
Attachment #8440188 -
Flags: review?(arthur.chen)
Comment 16•10 years ago
|
||
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
Attachment #8440188 -
Flags: review?(ferjmoreno) → review+
Comment 17•10 years ago
|
||
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)?
Flags: needinfo?(6a68)
Assignee | ||
Comment 18•10 years ago
|
||
Hi Arthur,
The way I generally test signups is to use an existing email address, like 6a68@mozilla.com, but append '+' and a random string to the end of the user part, like 6a68+123@mozilla.com. 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.
Flags: needinfo?(6a68)
Comment 20•10 years ago
|
||
Is the expected behavior like once I've verified the account, the item will then change to signed in as xxx@xxx.com? 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.
Flags: needinfo?(6a68)
Assignee | ||
Comment 21•10 years ago
|
||
(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 xxx@xxx.com? 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 :-(
Flags: needinfo?(6a68)
Assignee | ||
Comment 22•10 years ago
|
||
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?
Flags: needinfo?(arthur.chen)
Comment 23•10 years ago
|
||
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
Flags: needinfo?(arthur.chen)
Assignee | ||
Comment 24•10 years ago
|
||
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.
Flags: needinfo?(npark)
Comment 25•10 years ago
|
||
(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
Flags: needinfo?(npark)
Assignee | ||
Comment 26•10 years ago
|
||
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
Flags: needinfo?(arthur.chen)
Comment 27•10 years ago
|
||
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!
Attachment #8440188 -
Flags: review?(arthur.chen) → feedback+
Flags: needinfo?(arthur.chen)
Assignee | ||
Comment 28•10 years ago
|
||
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
Attachment #8440188 -
Flags: review?(arthur.chen)
Comment 29•10 years ago
|
||
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!
Attachment #8440188 -
Flags: review?(arthur.chen) → review+
Assignee | ||
Comment 30•10 years ago
|
||
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
Assignee | ||
Comment 31•10 years ago
|
||
Travis is green, except for a known marionette bug:
https://travis-ci.org/mozilla-b2g/gaia/builds/28361779
Merging to master.
Assignee | ||
Comment 32•10 years ago
|
||
doh, didn't actually click the green button earlier.
Master: https://github.com/mozilla-b2g/gaia/commit/aecb3041c71c5627aea30628ee82fc74e8af0d47
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v2.1:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S5 (4july)
Comment 33•10 years ago
|
||
Comment 34•10 years ago
|
||
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
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•