Open Bug 1335491 Opened 8 years ago Updated 2 years ago

FxA Login Tuning - Phase 1 - Only dismiss webview if account is verified

Categories

(Firefox for iOS :: Firefox Accounts, defect)

Other
iOS
defect

Tracking

()

Tracking Status
fxios-v6.0 --- affected
fxios-v6.1 --- fixed
fxios-v7.0 --- fixed
fxios 6.1 ---
fxios-v8.0 --- fixed

People

(Reporter: st3fan, Unassigned)

References

Details

(Whiteboard: [MobileCore])

Attachments

(2 files)

Phase 1 - Address immediate problem

1) On the server side, remove the delay when sending the "login"
   message. Instead of waiting 10 seconds, send it to us right away.

2) On the client side, we will look at the incoming login message and:
    - If verified=true then we store your account in the profile,
      dismiss the webview and start syncing. All set, good to go.
    - If verified=false then we store your account in the profile
      and keep the webview open, giving people plenty time to see
      the message about needing to find the verification email. We
      never close it automatically.

To support this, the following already exists in the client:

 - Sync runs periodically, so if verified flips to true at one point,
   sync will just start working.
 - We show a message in settings that your email is not verified.
 - Tapping that message or your account name in settings will open
   the page that talks about the verification email
This patch keeps the webview on screen if verified=false. This patch is very simple, which makes me a bit suspicious.
Assignee: nobody → sarentz
Attachment #8832164 - Flags: review?(sleroux) → review+
:sleroux, :st3fan: I remember at one point someone saying that Fx for iOS users update very quickly, so this might not be needed. 

To help users that don't upgrade Fx, could we use a new `contextyou two be amenable to open FxA with a new `context` query parameter so that we know to send the `login` message immediately and the browser will "Do The Right Thing", and if the old context is used, we still send `login` after a short delay?

We could use `context=fx_ios_v2`.

If the WebView
Ugh, still getting used to this new keyboard. I caused a submit too early. Anyways, could we use `fx_ios_v2` to indicate the browser will do the right thing when it receives the `login` message?

The second part that was cut off - if the WebView is still displayed after the login message is received, should FxA start polling to transition to the "Your email has been verified" screen once verification is complete, or will Fx for iOS take over and show its own UI at that point?
Flags: needinfo?(sleroux)
Flags: needinfo?(sarentz)
> sleroux, :st3fan: I remember at one point someone saying that Fx for iOS users update very quickly, so this might not be needed. 

I would say the majority of users upgrade pretty quickly because of iOS's automatic app updates so this won't be that much of a concern.

> To help users that don't upgrade Fx, could we use a new `contextyou two be amenable to open FxA with a new `context` query parameter so that we know to send the `login` message immediately and the browser will "Do The Right Thing", and if the old context is used, we still send `login` after a short delay?
> We could use `context=fx_ios_v2`.

I might be a bit confused but in order for us to send the new context parameter we would have to update code since the url is coded into the client. 

> The second part that was cut off - if the WebView is still displayed after the login message is received, should FxA start polling to transition to the "Your email has been verified" screen once verification is complete, or will Fx for iOS take over and show its own UI at that point?

IIUC (:st3fan correct me otherwise), when the app goes to do it's periodic sync, we will learn about the account being verified and update the client so I guess in a sense the client uses the syncing as a polling mechanism. I can see having polling from the FxA server be beneficial in the case where the user is staring at the confirm email screen, backgrounds the app, checks their email, and goes back. This way they would have a visual confirmation that the account has been verified instead of relying on the sync to do it in the background.

I assume the limitation (for now) with the polling is the user needs to be on the web screen for the polling callback to fire. When push is implemented (https://bugzilla.mozilla.org/show_bug.cgi?id=1335503) this limitation is gone and we can listen for the callback app-wide.
Flags: needinfo?(sleroux)
(In reply to Stephan Leroux [:sleroux] from comment #5)
...
> IIUC (:st3fan correct me otherwise), when the app goes to do it's periodic
> sync, we will learn about the account being verified and update the client
> so I guess in a sense the client uses the syncing as a polling mechanism.

Correct.

> I
> can see having polling from the FxA server be beneficial in the case where
> the user is staring at the confirm email screen, backgrounds the app, checks
> their email, and goes back. This way they would have a visual confirmation
> that the account has been verified instead of relying on the sync to do it
> in the background.

There are two places where polling can happen:

- Firefox can poll periodically while that webview is open to find out if the account flipped to verified.

- The webview (fxa content) can poll the fxa server on its own to find out if the account is verified, and then notify Firefox through a similar message that we do now.

The first already sort of exists in our app. Except that it runs not very frequently. The second is implemented in Desktop if I understand correctly? I am not sure what the best route it.

> I assume the limitation (for now) with the polling is the user needs to be
> on the web screen for the polling callback to fire. When push is implemented
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1335503) this limitation is
> gone and we can listen for the callback app-wide.

What is 'the polling callback' ?

I filed bug 1335503 for listening to the push notification. I will also file a bug for polling as an alternative. Not sure which one should have priority.
Flags: needinfo?(sarentz)
(In reply to Stefan Arentz [:st3fan] from comment #6)
> 
> There are two places where polling can happen:
> 
> - Firefox can poll periodically while that webview is open to find out if
> the account flipped to verified.
> 
> - The webview (fxa content) can poll the fxa server on its own to find out
> if the account is verified, and then notify Firefox through a similar
> message that we do now.
> 
> The first already sort of exists in our app. Except that it runs not very
> frequently. The second is implemented in Desktop if I understand correctly?
> I am not sure what the best route it.
> 


When we implemented the Fx for iOS code, we didn't know that the browser would take over the UI
whenever it received a `login` message, so the good news is the content server already polls as
long as the "confirm your email" screen is displayed. We don't send any additional 
`account is verified` messages because we expect the browser to find out within a few seconds of us
that the account is verified.
Landed on v6.x

https://github.com/mozilla-mobile/firefox-ios/commit/c1d474d18d71d7dcb88e858e2873a4dbcd0f79a6

Will need 'downlift' when Swift 3.0 migration has finished. WHich is why I am leaving the bug open.
Flags: needinfo?(sarentz)
Flags: needinfo?(sarentz)
Whiteboard: needsdownlift
FYI I've deployed a BuddyBuild of 6.1 containing this fix for verification. I've sent it to the existing Nightly test group but if there is anyone else on this thread who is interested in testing the build let me know.
This is essentially the same PR we already landed on v6.x but now converted to Swift 3.0 so that it can land on master and uplifted to v7.x.
Attachment #8841666 - Flags: review?(sleroux)
Whiteboard: needsdownlift → needsuplift
master 241395c334241abf3ddbf2ba10d0de642dbaff3a
Whiteboard: needsuplift → [needsuplift]
Attachment #8841666 - Flags: review?(sleroux) → review+
v7.x c9ee1acb7dbbea24cd3bb566f7a04780c22e01ef
Whiteboard: [needsuplift] → [MobileCore]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: sarentz → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.