Skip to content
This repository has been archived by the owner. It is now read-only.

fix(iOS): Send the login message within the visibilitychange #4682

Merged
merged 4 commits into from Feb 6, 2017

Conversation

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jan 27, 2017

For verified accounts, the login message is sent immediately.

For unverified accounts, the login message is sent after the
first of 1) a 5 second timeout, or 2) a blur DOM event.

The blur DOM event is to handle the case where a user receives
their verification email and opens the email client before the timeout
has occurred.

For Fx for iOS >= 6.1, always send the login message immediately.

fixes #4377

@shane-tomlinson shane-tomlinson changed the title fix(iOS): Send the login message within the visibilitychange WIP fix(iOS): Send the login message within the visibilitychange Jan 27, 2017
@shane-tomlinson shane-tomlinson force-pushed the issue-4377-send-login-on-visibilitychange branch from d42b31d to 8635bda Jan 31, 2017
For verified accounts, the `login` message is sent immediately.

For unverified accounts, the `login` message is sent after the
first of 1) a 5 second timeout, or 2) a `blur` DOM event.

The `blur` DOM event is to handle the case where a user receives
their verification email and opens the email client before the timeout
has occurred.

issue #4377
@shane-tomlinson shane-tomlinson force-pushed the issue-4377-send-login-on-visibilitychange branch from 8635bda to bf709a5 Feb 1, 2017
@shane-tomlinson shane-tomlinson changed the title WIP fix(iOS): Send the login message within the visibilitychange WIP fix(iOS): Send the login in more cases to Fx for iOS. Feb 1, 2017
@shane-tomlinson shane-tomlinson requested a review from vbudhram Feb 2, 2017
@shane-tomlinson shane-tomlinson changed the title WIP fix(iOS): Send the login in more cases to Fx for iOS. fix(iOS): Send the login in more cases to Fx for iOS. Feb 2, 2017
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 2, 2017

@vbudhram - mind doing a review of this, even before @st3fan has reported back his tests? I'd like to have the ducks lined up if he says "yeah, works as expected."

@shane-tomlinson shane-tomlinson changed the title fix(iOS): Send the login in more cases to Fx for iOS. WIP fix(iOS): Send the login message within the visibilitychange Feb 2, 2017
@shane-tomlinson shane-tomlinson changed the title WIP fix(iOS): Send the login message within the visibilitychange fix(iOS): Send the login message within the visibilitychange Feb 2, 2017
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 2, 2017

This code is running on https://stomlinson.dev.lcip.org. I used the config from https://gist.github.com/shane-tomlinson/ac0e30442ebd9f32b13b449239319f60 to test on device. Unfortunately, my device doesn't have @st3fan's patch, so I haven't tested in full.

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 2, 2017

fixes #4377

@vbudhram
Copy link
Contributor

@vbudhram vbudhram commented Feb 2, 2017

@shane-tomlinson On it 👍, I can also test with the ios patch and make sure all is well.

Copy link
Contributor

@vbudhram vbudhram left a comment

Tested this with the following use cases

FxiOS 6.0

  • Sign-in with account verified
    • Sends login message and redirects to settings page
  • Sign-in with unverified account, minimize app, click verify email, reopens app
    • Waits 10 seconds, sends login, redirects to settings

FxiOS 6.1 (iOS Patch version)

  • Sign-in with account verified
    • Sends login message and redirects to settings page
  • Sign-in with unverified account, minimize app, click verify email, reopens app
    • Waits indefinitely at confirm sign-in, clicking back button shows verified user

@shane-tomlinson This LGTM, r+. Something to note with the patched iOS version, after returning to the app from verifying email, the screen will still be on the confirm sign-in page even though they just verified, which could lead to come confusion.

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 3, 2017

redirects to settings

Agh! That's not supposed to happen! It's supposed to redirect to /signup_complete I thought!

Fx for iOS allows the user to see the "confirm your email" screen,
but never takes it away after the user verifies. Allow the poll
so that the user sees the "Signup complete!" screen after they
verify their email.
@shane-tomlinson shane-tomlinson force-pushed the issue-4377-send-login-on-visibilitychange branch from 8319301 to 3135438 Feb 3, 2017
@vbudhram
Copy link
Contributor

@vbudhram vbudhram commented Feb 3, 2017

@shane-tomlinson Updates look r+! This performs exactly as we talked about.

FxiOS 6.1 >=

  • Polls until account is verified and then transitions to /signup_complete. User has to hit the top navigation settings button to get back to FxiOS settings screen. The user will show as connected in this page.

Merge at will!

@shane-tomlinson shane-tomlinson merged commit 5bca42f into master Feb 6, 2017
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 98.518%
Details
@shane-tomlinson shane-tomlinson deleted the issue-4377-send-login-on-visibilitychange branch Feb 6, 2017
@rfk rfk added this to the FxA-0: quality milestone Feb 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants