Bug 1200886 - add tooltips to the Sync buttons which reflect the current state of Sync. r=adw

browser/base/content/browser-syncui.js

@@ -125,6 +125,21 @@ let gSyncUI = {
            firstSync == "notReady";
   },
 
+  _needsVerification() {
+    // For callers who care about the distinction between "needs setup" and
+    // "setup but needs verification"
+    // See _needsSetup for the subtleties here.
+    if (Weave.Status._authManager._signedInUser === undefined) {
+      // a legacy sync user - no "verified" concept there.
+      return false;
+    }
+    if (!Weave.Status._authManager._signedInUser) {
+      // no user configured at all, so not in a "need verification" state.
+      return false;
+    }
+    return !Weave.Status._authManager._signedInUser.verified;
+  },
+
   // Note that we don't show login errors in a notification bar here, but do
   // still need to track a login-failed state so the "Tools" menu updates
   // with the correct state.
@@ -154,21 +169,7 @@ let gSyncUI = {
       document.getElementById("sync-syncnow-state").hidden = false;
     }
 
-    if (!gBrowser)
-      return;
-
-    let syncButton = document.getElementById("sync-button");
-    let statusButton = document.getElementById("PanelUI-fxa-icon");
-    if (needsSetup) {
-      if (syncButton) {
-        syncButton.removeAttribute("tooltiptext");
-      }
-      if (statusButton) {
-        statusButton.removeAttribute("tooltiptext");
-      }
-    }
-
-    this._updateLastSyncTime();
+    this._updateSyncButtonsTooltip();
   },
 
   // Functions called by observers
@@ -311,39 +312,60 @@ let gSyncUI = {
     gFxAccounts.openSignInAgainPage(entryPoint);
   },
 
-  // Helpers
-  _updateLastSyncTime: function SUI__updateLastSyncTime() {
+  /* Update the tooltip for the Sync Toolbar button and the Sync spinner in the
+     FxA hamburger area.
+     If Sync is configured, the tooltip is when the last sync occurred,
+     otherwise the tooltip reflects the fact that Sync needs to be
+     (re-)configured.
+  */
+  _updateSyncButtonsTooltip: function() {
     if (!gBrowser)
       return;
 
     let syncButton = document.getElementById("sync-button");
     let statusButton = document.getElementById("PanelUI-fxa-icon");
 
-    let lastSync;
+    let email;
     try {
-      lastSync = new Date(Services.prefs.getCharPref("services.sync.lastSync"));
-    }
-    catch (e) { };
-    if (!lastSync || this._needsSetup()) {
-      if (syncButton) {
-        syncButton.removeAttribute("tooltiptext");
-      }
-      if (statusButton) {
-        statusButton.removeAttribute("tooltiptext");
-      }
-      return;
-    }
+      email = Services.prefs.getCharPref("services.sync.username");
+    } catch (ex) {}
 
-    // Show the day-of-week and time (HH:MM) of last sync
-    let lastSyncDateString = lastSync.toLocaleFormat("%a %H:%M");
-    let lastSyncLabel =
-      this._stringBundle.formatStringFromName("lastSync2.label", [lastSyncDateString], 1);
-
-    if (syncButton) {
-      syncButton.setAttribute("tooltiptext", lastSyncLabel);
+    // This is a little messy as the Sync buttons are 1/2 Sync related and
+    // 1/2 FxA related - so for some strings we use Sync strings, but for
+    // others we reach into gFxAccounts for strings.
+    let tooltiptext;
+    if (this._needsVerification()) {
+      // "needs verification"
+      tooltiptext = gFxAccounts.strings.formatStringFromName("verifyDescription", [email], 1);
+    } else if (this._needsSetup()) {
+      // "needs setup".
+      tooltiptext = this._stringBundle.GetStringFromName("signInToSync.description");
+    } else if (this._loginFailed()) {
+      // "need to reconnect/re-enter your password"
+      tooltiptext = gFxAccounts.strings.formatStringFromName("reconnectDescription", [email], 1);
+    } else {
+      // Sync appears configured - format the "last synced at" time.
+      try {
+        let lastSync = new Date(Services.prefs.getCharPref("services.sync.lastSync"));
+        // Show the day-of-week and time (HH:MM) of last sync
+        let lastSyncDateString = lastSync.toLocaleFormat("%a %H:%M");
+        tooltiptext = this._stringBundle.formatStringFromName("lastSync2.label", [lastSyncDateString], 1);
+      }
+      catch (e) {
+        // pref doesn't exist (which will be the case until we've seen the
+        // first successful sync) or is invalid (which should be impossible!)
+        // Just leave tooltiptext as the empty string in these cases, which
+        // will cause the tooltip to be removed below.
+      }
     }
-    if (statusButton) {
-      statusButton.setAttribute("tooltiptext", lastSyncLabel);
+    for (let button of [syncButton, statusButton]) {
+      if (button) {
+        if (tooltiptext) {
+          button.setAttribute("tooltiptext", tooltiptext);
+        } else {
+          button.removeAttribute("tooltiptext");
+        }
+      }
     }
   },
 

browser/base/content/test/general/browser_syncui.js

@@ -34,23 +34,60 @@ function promiseObserver(topic) {
   });
 }
 
+function checkButtonTooltips(stringPrefix) {
+  for (let butId of ["sync-button", "PanelUI-fxa-icon"]) {
+    let text = document.getElementById(butId).getAttribute("tooltiptext");
+    let desc = `Text is "${text}", expecting it to start with "${stringPrefix}"`
+    Assert.ok(text.startsWith(stringPrefix), desc);
+  }
+}
+
 add_task(function* prepare() {
+  // add the Sync button to the toolbar so we can get it!
+  CustomizableUI.addWidgetToArea("sync-button", CustomizableUI.AREA_NAVBAR);
+  registerCleanupFunction(() => {
+    CustomizableUI.removeWidgetFromArea("sync-button");
+  });
+
   let xps = Components.classes["@mozilla.org/weave/service;1"]
                               .getService(Components.interfaces.nsISupports)
                               .wrappedJSObject;
   yield xps.whenLoaded();
+  // Put Sync and the UI into a known state.
+  Weave.Status.login = Weave.LOGIN_FAILED_NO_USERNAME;
+  Services.obs.notifyObservers(null, "weave:ui:clear-error", null);
+
   checkBroadcasterVisible("sync-setup-state");
+  checkButtonTooltips("Sign In To Sync");
   // mock out the "_needsSetup()" function so we don't short-circuit.
   let oldNeedsSetup = window.gSyncUI._needsSetup;
   window.gSyncUI._needsSetup = () => false;
   registerCleanupFunction(() => {
     window.gSyncUI._needsSetup = oldNeedsSetup;
+    // and an observer to set the state back to what it should be now we've
+    // restored the stub.
+    Services.obs.notifyObservers(null, "weave:ui:clear-error", null);
   });
   // and a notification to have the state change away from "needs setup"
   Services.obs.notifyObservers(null, "weave:ui:clear-error", null);
   checkBroadcasterVisible("sync-syncnow-state");
 });
 
+add_task(function* testSyncNeedsVerification() {
+  Assert.equal(Notifications.notifications.length, 0, "start with no notifications");
+  // mock out the "_needsVerification()" function
+  let oldNeedsVerification = window.gSyncUI._needsVerification;
+  window.gSyncUI._needsVerification = () => true;
+  try {
+    // a notification for the state change
+    Services.obs.notifyObservers(null, "weave:ui:clear-error", null);
+    checkButtonTooltips("Verify");
+  } finally {
+    window.gSyncUI._needsVerification = oldNeedsVerification;
+  }
+});
+
+
 add_task(function* testSyncLoginError() {
   Assert.equal(Notifications.notifications.length, 0, "start with no notifications");
   checkBroadcasterVisible("sync-syncnow-state");
@@ -63,6 +100,8 @@ add_task(function* testSyncLoginError() {
   Assert.equal(Notifications.notifications.length, 0, "no notifications shown on login error");
   // But the menu *should* reflect the login error.
   checkBroadcasterVisible("sync-reauth-state");
+  // The tooltips for the buttons should also reflect it.
+  checkButtonTooltips("Reconnect");
 
   // Now pretend we just had a successful login - the error notification should go away.
   Weave.Status.sync = Weave.STATUS_OK;
@@ -96,27 +135,36 @@ function testButtonActions(startNotification, endNotification) {
   checkButtonsStatus(false);
 }
 
-add_task(function* testButtonActivities() {
-  // add the Sync button to the panel so we can get it!
+function doTestButtonActivities() {
+  testButtonActions("weave:service:login:start", "weave:service:login:finish");
+  testButtonActions("weave:service:login:start", "weave:service:login:error");
+
+  testButtonActions("weave:service:sync:start", "weave:service:sync:finish");
+  testButtonActions("weave:service:sync:start", "weave:service:sync:error");
+
+  // and ensure the counters correctly handle multiple in-flight syncs
+  Services.obs.notifyObservers(null, "weave:service:sync:start", null);
+  checkButtonsStatus(true);
+  // sync stops.
+  Services.obs.notifyObservers(null, "weave:service:sync:finish", null);
+  // Button should not be active.
+  checkButtonsStatus(false);
+}
+
+add_task(function* testButtonActivitiesInNavBar() {
+  // check the button's functionality while the button is in the NavBar - which
+  // it already is.
+  doTestButtonActivities();
+});
+
+add_task(function* testButtonActivitiesInPanel() {
+  // check the button's functionality while the button is in the panel - it's
+  // currently in the NavBar - move it to the panel and open it.
   CustomizableUI.addWidgetToArea("sync-button", CustomizableUI.AREA_PANEL);
-  // check the button's functionality
   yield PanelUI.show();
   try {
-    testButtonActions("weave:service:login:start", "weave:service:login:finish");
-    testButtonActions("weave:service:login:start", "weave:service:login:error");
-
-    testButtonActions("weave:service:sync:start", "weave:service:sync:finish");
-    testButtonActions("weave:service:sync:start", "weave:service:sync:error");
-
-    // and ensure the counters correctly handle multiple in-flight syncs
-    Services.obs.notifyObservers(null, "weave:service:sync:start", null);
-    checkButtonsStatus(true);
-    // sync stops.
-    Services.obs.notifyObservers(null, "weave:service:sync:finish", null);
-    // Button should not be active.
-    checkButtonsStatus(false);
+    doTestButtonActivities();
   } finally {
     PanelUI.hide();
-    CustomizableUI.removeWidgetFromArea("sync-button");
   }
 });

services/sync/locales/en-US/sync.properties

@@ -8,6 +8,10 @@ client.name2 = %1$S's %2$S on %3$S
 # %S is the date and time at which the last sync successfully completed
 lastSync2.label = Last sync: %S
 
+# signInToSync.description is the tooltip for the Sync buttons when Sync is
+# not configured.
+signInToSync.description = Sign In To Sync
+
 mobile.label = Mobile Bookmarks
 
 remote.pending.label = Remote tabs are being synced…