957436 - Panel or notification for first fxa-based sync

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

The current mockups for fxa-based sync calls for a panel anchored to the hamburger as soon as you sign in to Firefox Accounts and sync starts.  The wording is something like:  "Your browser will now being syncing all your stuff", with a progress indicator and a "customize" button, which displays sync options.

On one hand, it might be possible to defer this until post-29 - no such panel exists currently.  On the other hand, the current mockups for the new signup flows do *not* show the sync options by default - it just starts syncing - so this might be the only user-visible way to configure sync.  On the third hand, this panel is transient (as is the nature of panels) and presumably will not be shown every time we sync, so relying on this as the main entry-point to sync options doesn't seem sensible.

This needs more thought.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Some discussion about this here ("Walkthru of FxA/Sync Account Creation"):
https://mail.mozilla.org/pipermail/sync-dev/2014-January/thread.html#645

Comment 2

[:Gavin Sharp [email: [e-mail]]]

https://wiki.mozilla.org/User_Services/Sync/Datatype_Selection_Fx29 describes the agreed-upon desired behavior in more detail.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Tim's happy to take this

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0006-Bug-957436-Show-doorhanger-for-first-fxa-based-sync.patch

Sorry for the OSX-only CSS, it's late...

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8364083 [details] [diff] [review]
0006-Bug-957436-Show-doorhanger-for-first-fxa-based-sync.patch

Review of attachment 8364083 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser-fxaccounts.js
@@ +35,5 @@
>      }
>    },
>  
> +  observe: function (subject, topic) {
> +    if (topic == "fxaccounts:onverified") {

A problem here might be that a user might sign in and be verified even though it is not their first-time sync.  Seeing it's not clear what we do in this case, I think we can ignore that for now and fix that later if necessary.

Also, this will need retesting now that bug 952063 has landed.

Also also: The UX flows call for a modal dialog to (optionally) be displayed after initial signin, which displays sync prefs and prevents sync from actually synching until it is dismissed.  Sadly that patch hasn't been written yet, but I'm fairly sure that this panel will only be displayed *after* that dialog has been dismissed.  So the condition of when we display the panel will probably need to be tweaked (eg, I'm thinking that if that dialog is to be shown, we will temporarily disable sync until it is dismissed - so this condition will probably need to include something like "is sync configured and enabled"

::: browser/base/content/browser.css
@@ +894,5 @@
>    transition-duration: 1ms;
>    transition-timing-function: linear;
>  }
>  
> +

3 blank lines seems overkill?

::: services/fxaccounts/FxAccounts.jsm
@@ +394,5 @@
>                data.verified = true;
>                return this.setUserAccountData(data);
>              })
>              .then((data) => {
> +              // Notify everyone that the user is now verified.

bug 952063 is going to obsolete this change and the one in FxAccountsCommon.js

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0004-Bug-957436-Show-doorhanger-for-first-fxa-based-sync.patch, v2

(In reply to Mark Hammond [:markh] from comment #5)
> > +  observe: function (subject, topic) {
> > +    if (topic == "fxaccounts:onverified") {
> 
> A problem here might be that a user might sign in and be verified even
> though it is not their first-time sync.  Seeing it's not clear what we do in
> this case, I think we can ignore that for now and fix that later if
> necessary.

With the changes from bug 952063, onverified is only sent when the keys have been fetched and unwrapped which should only happen once, right?

> Also, this will need retesting now that bug 952063 has landed.

Re-tested :)

> Also also: The UX flows call for a modal dialog to (optionally) be displayed
> after initial signin, which displays sync prefs and prevents sync from
> actually synching until it is dismissed.  Sadly that patch hasn't been
> written yet, but I'm fairly sure that this panel will only be displayed
> *after* that dialog has been dismissed.  So the condition of when we display
> the panel will probably need to be tweaked (eg, I'm thinking that if that
> dialog is to be shown, we will temporarily disable sync until it is
> dismissed - so this condition will probably need to include something like
> "is sync configured and enabled"

AFAIU the UX flow we only show the doorhanger if the user did not tick off "customize sync" when registering. If they chose to customize they are going to see a modal dialog with all the sync options. So they either see on or the other, no?

> >              .then((data) => {
> > +              // Notify everyone that the user is now verified.
> 
> bug 952063 is going to obsolete this change and the one in
> FxAccountsCommon.js

Indeed, removed those changes.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/45f1ee0a3a1c

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Pushed a follow-up to fix leaks on Linux. Turns out .uninit() can be called without .init() ever being called when a window is opened and closed really quickly.

https://hg.mozilla.org/integration/fx-team/rev/de3521df8d43

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/45f1ee0a3a1c
https://hg.mozilla.org/mozilla-central/rev/de3521df8d43