926709 - Replace UI entry points for sync setup with opening about:accounts

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Replace UI entry points for sync setup with opening about:accounts

Comment 2

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

Comment on attachment 816869 [details] [diff] [review]
Replace UI entry points for sync setup with opening about:accounts

I think we want switchToTabHavingURI-like behavior for about:accounts, similar to about:addons, so we'll have to take that into account.

Can you get rid of the openSetups now?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Replace UI entry points for sync setup with opening about:accounts, v2

This patch removes most (all?) of the sync wizard. The only thing of the sync setup we're keeping is about:sync-progress because we can easily reuse that.

Comment 4

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

Comment on attachment 817968 [details] [diff] [review]
Replace UI entry points for sync setup with opening about:accounts, v2

>diff --git a/browser/base/content/browser-syncui.js b/browser/base/content/browser-syncui.js

>+  openAccountsPage: function () {
>+    window.switchToTabHavingURI("about:accounts", true);
>+  },

"window." is a bit unusual...

>-        function() { gSyncUI.openQuotaDialog(); return true; } )
>+        function() { gSyncUI.openAccountsPage(); return true; } )

As mentioned IRL, we should get a bug on file to make sure the accounts page handles quota stuff, if we're linking to it for that.

>diff --git a/browser/components/preferences/in-content/sync.js b/browser/components/preferences/in-content/sync.js

(same comments apply to non-in-content versions, obviously)

>-  needsUpdate: function () {
>-    this.page = PAGE_NEEDS_UPDATE;

PAGE_NEEDS_UPDATE is now unused.

>   updateWeavePrefs: function () {
>     if (Weave.Status.service == Weave.CLIENT_NOT_CONFIGURED ||
>         Weave.Svc.Prefs.get("firstSync", "") == "notReady") {
>       this.page = PAGE_NO_ACCOUNT;
>-    } else if (Weave.Status.login == Weave.LOGIN_FAILED_INVALID_PASSPHRASE ||
>-               Weave.Status.login == Weave.LOGIN_FAILED_LOGIN_REJECTED) {
>-      this.needsUpdate();
>-    } else {
>+    } else if (Weave.Status.login != Weave.LOGIN_FAILED_INVALID_PASSPHRASE &&
>+               Weave.Status.login != Weave.LOGIN_FAILED_LOGIN_REJECTED) {
>       this.page = PAGE_HAS_ACCOUNT;
>       document.getElementById("accountName").value = Weave.Service.identity.account;
>-      document.getElementById("syncComputerName").value = Weave.Service.clientsEngine.localName;
>       document.getElementById("tosPP").hidden = this._usingCustomServer;
>     }

This leaves the "INVALID_PASSPHRASE" and "LOGIN_REJECTED" cases unhandled, right? Seems like they should just show PAGE_NO_ACCOUNT for now.

>diff --git a/browser/components/preferences/in-content/sync.xul b/browser/components/preferences/in-content/sync.xul

>   <vbox id="hasAccount">

>-        <button type="menu"
>-                label="&manageAccount.label;"
>-                accesskey="&manageAccount.accesskey;">

We'll probably want a link to about:accounts in this pane somewhere?

>-            <label value="&syncDeviceName.label;"
>-                   accesskey="&syncDeviceName.accesskey;"
>-                   control="syncComputerName"/>
>-            <textbox id="syncComputerName"
>-                     onchange="gSyncUtils.changeName(this)"/>

Do we really want to remove this?

r=me for elm with those addressed.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/elm/rev/6774532951f6

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Oh, forgot we can set this to fixed as we'll not merge it to m-c for now.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6774532951f6

Comment 8

[James Bonacci [:jbonacci]]

Cleaning up Resolved/Fixed bugs from December's first release.
Verified that we now have a working first-release of FxA to Desktop/Android Nightly.
Re-open as needed.