539057 - better flow with start over

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Mike Connor [:mconnor]]

Need to allow merge/overwrite without logging back in.  Also need to make it more clear what will happen if you want to use a different account.

Comment 1

[Ragavan S [:rags]]

Actually, I think we should only allow one account per profile. If someone wants to set up another weave account, then it should be on a different profile.

It seems like the right long term solution to the use a different account problem.

Comment 2

[Mike Connor [:mconnor]]

Except that profile manager is officially unsupported (since before 1.0), and there's talk of removing it completely.  Also, what if you close your account, and then want to sign up for a new one?  Without Weave, you're basically at the "don't use Weave" or "manually migrate your data"

Comment 3

[WildcatRay]

Some things I would like to see in "Start Over":

1. If there is already login, password, & passphrase in the client profile, have the appropriate fields in Start Over be populated with them.

Background: When making a new profile and having to set up Weave in it, I have had to go to my password keeper where I have my login data and copy/paste them in even though I have copied signons.sqlite and key3.db from a known good profile into the new profile. Things are a bit more convenient if the Saved Passwords add-on is installed as this allows the user to access their saved login data from within the browser.

FYI: I have a very strong password and passphrase that I generated in my password keeper, KeePass Password Safe.

2. Choose what items to sync before starting to sync with the server.

Background: I do not want to sync prefs as I have some different prefs in different clients. To not have this happen, I must set up Weave, exit the setup, go back into the Weave prefs, uncheck Sync Prefs, close Weave prefs and close the browser as I cannot be sure that Weave will not sync prefs at that point unless I do that.

If you prefer, I can file new bugs for, I would assume, each unless they are already filed. Thanks.

Comment 4

[Ed Lee :Mardak]

I believe this bug is about the re-logging in aspect for the sync direction, and bug 537897 is to choose what to sync.

Comment 5

[WildcatRay]

(In reply to comment #3)
> Some things I would like to see in "Start Over":
> 
> 1. If there is already login, password, & passphrase in the client profile,
> have the appropriate fields in Start Over be populated with them.
> 
> Background: When making a new profile and having to set up Weave in it, I have
> had to go to my password keeper where I have my login data and copy/paste them
> in even though I have copied signons.sqlite and key3.db from a known good
> profile into the new profile. Things are a bit more convenient if the Saved
> Passwords add-on is installed as this allows the user to access their saved
> login data from within the browser.
> 

Would I be right say this needs to be filed as a separate bug?

> FYI: I have a very strong password and passphrase that I generated in my
> password keeper, KeePass Password Safe.
> 
> 2. Choose what items to sync before starting to sync with the server.
> 
> Background: I do not want to sync prefs as I have some different prefs in
> different clients. To not have this happen, I must set up Weave, exit the
> setup, go back into the Weave prefs, uncheck Sync Prefs, close Weave prefs and
> close the browser as I cannot be sure that Weave will not sync prefs at that
> point unless I do that.
> 

As per Edward in Comment #4, my Item 2 is Bug 537897.

Thanks, Edward.

Comment 6

[Mike Connor [:mconnor]]

1. is a separate bug, yes.

Comment 7

[WildcatRay]

(In reply to comment #6)
> 1. is a separate bug, yes.

Filed Bug 548346 Populate Login, Password & Passphrase Fields If Values Exist in Profile When Using Start Over

Comment 8

[Mike Connor [:mconnor]]

We can probably add the UI piece from the wizard to the confirmation page, there's vertical space.

Comment 9

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

So I think this does it. I used a different entity because startOver is also used on sync type page, which we should probably rethink (since it starts over, but you can now jump right to that page). I also just came up with strings pretty quickly, so better wording is welcome.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 434642 [details] [diff] [review]
Patch v0.1

Okay, so, in the course of triage I identified a few things that should get fixed here (and we need to string freeze in the next week, until after 1.3).

Need to add:

* On use a different account
** Clear out the password/passphrase stuff in pwmgr
** Confirm this action (this is going to be rare, but as it's destructive-ish, we shoud confirm)
*** "Do you want to use a different account? This will reset all of your sync account information and preferences. [Use a Different Account] [Cancel] (dialog widget will take care of button ordering)
* All cases
** Reuse the same UI that we have in the end of the wizard for setting what to sync, on the confirmation page

>+  resetSync: function() {
>+    this.handleExpanderClick();
>+    Weave.Svc.Prefs.set("firstSync", "notReady");
>+    this.updateWeavePrefs();
>+  },

This bit is fine.

>diff --git a/source/chrome/content/preferences/fx-prefs.xul b/source/chrome/content/preferences/fx-prefs.xul
>--- a/source/chrome/content/preferences/fx-prefs.xul
>+++ b/source/chrome/content/preferences/fx-prefs.xul
>@@ -320,17 +320,20 @@
>                       <label class="text-link"
>                              onclick="gWeavePane.changePassword(); return false;"
>                              value="&changePassword.label;"/>
>                       <label class="text-link"
>                              onclick="gWeavePane.changePassphrase(); return false;"
>                              value="&changePassphrase.label;"/>
>                       <label class="text-link"
>                              onclick="gWeavePane.startOver(); return false;"
>-                             value="&startOver.label;"/>
>+                             value="&differentAccount.label;"/>
>+                      <label class="text-link"
>+                             onclick="gWeavePane.resetSync(); return false;"
>+                             value="&resetSync.label;"/>

Let's leave the different account bit for last.

>+<!ENTITY differentAccount.label       "Use a Different Account">
>+<!ENTITY resetSync.label              "Reset Sync Option">

Just "Reset Sync" is more correct here.

Comment 11

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2 (WIP)

Addresses most of comment #10, except for
> ** Reuse the same UI that we have in the end of the wizard for setting what to
> sync, on the confirmation page

Also, I'm still looking for an answer to this - it's a bit awkward, now even more so with the dialog (though the dialog would need to be there since it is that destructive action)
> I used a different entity because startOver is also
> used on sync type page, which we should probably rethink (since it starts over,
> but you can now jump right to that page).

And finally... the dialog is awkward as I made it, "different account" is in there 3 times. I could drop the title? Turn it into a yes/no dialog?

Comment 12

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.3

Per discussion on IRC, dropped the whole options in the final screen.

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 435535 [details] [diff] [review]
Patch v0.3

per discussion on IRC, new patch is expected today.

Comment 14

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.4

wipeCredentials --> startOver and made more useful.

Also, note that this doesn't completely fix bug 545958. It fixes the primary problem of being signed in, but not the secondary problem of skipping the passphrase step.

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 436223 [details] [diff] [review]
Patch v0.4

rock.

Comment 16

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/labs/weave/rev/ab62101dbcbe
* Adds an option to the manage account to change sync option.
* Changes "start over" to "use a different account".
* Hides the "start over" button on the sync options page when not running through the whole process.
* Adds a prompt when starting over (which fixes bug 524186).

Comment 17

[Tony Chung [:tchung]]

Flagging in-litmus?.  this could use a litmus testcase for this area

Comment 18

[Tracy Walker [:tracy]]

verified with 1.3b5

Comment 19

[Tracy Walker [:tracy]]

added test cases https://litmus.mozilla.org/show_test.cgi?id=13770