Bug 1269348 - Show last sync date tooltip on Synced Tabs sidebar device names. r=markh draft
authorEdouard Oger <eoger@fastmail.com>
Wed, 18 May 2016 15:45:29 -0700
changeset 368495 716eaa7e633a53a7a8f4e3de0070f7003bec1c0c
parent 368365 c4449eab07d39e20ea315603f1b1863eeed7dcfe
child 521298 964743d1806befbd4a234e92b473c653331e5438
push id18561
push userbmo:edouard.oger@gmail.com
push dateWed, 18 May 2016 22:46:24 +0000
reviewersmarkh
bugs1269348
milestone49.0a1
Bug 1269348 - Show last sync date tooltip on Synced Tabs sidebar device names. r=markh MozReview-Commit-ID: LkGNdSF8Znm
browser/base/content/browser-syncui.js
browser/base/content/test/general/browser_syncui.js
browser/components/syncedtabs/TabListView.js
services/sync/modules/SyncedTabs.jsm
services/sync/modules/engines/tabs.js
services/sync/tests/unit/test_tab_store.js
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -398,20 +398,17 @@ var gSyncUI = {
       tooltiptext = this._stringBundle.GetStringFromName("signInToSync.description");
     } else if (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.toLocaleDateString(undefined,
-          {weekday: 'long', hour: 'numeric', minute: 'numeric'});
-        tooltiptext = this._stringBundle.formatStringFromName("lastSync2.label", [lastSyncDateString], 1);
+        tooltiptext = this.formatLastSyncDate(lastSync);
       }
       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.
       }
     }
@@ -426,16 +423,34 @@ var gSyncUI = {
       if (tooltiptext) {
         broadcaster.setAttribute("tooltiptext", tooltiptext);
       } else {
         broadcaster.removeAttribute("tooltiptext");
       }
     }
   }),
 
+  formatLastSyncDate: function(date) {
+    let dateFormat;
+    let sixDaysAgo = (() => {
+      let date = new Date();
+      date.setDate(date.getDate() - 6);
+      date.setHours(0, 0, 0, 0);
+      return date;
+    })();
+    // It may be confusing for the user to see "Last Sync: Monday" when the last sync was a indeed a Monday but 3 weeks ago
+    if (date < sixDaysAgo) {
+      dateFormat = {month: 'long', day: 'numeric'};
+    } else {
+      dateFormat = {weekday: 'long', hour: 'numeric', minute: 'numeric'};
+    }
+    let lastSyncDateString = date.toLocaleDateString(undefined, dateFormat);
+    return this._stringBundle.formatStringFromName("lastSync2.label", [lastSyncDateString], 1);
+  },
+
   onSyncFinish: function SUI_onSyncFinish() {
     let title = this._stringBundle.GetStringFromName("error.sync.title");
   },
 
   onClientsSynced: function() {
     let broadcaster = document.getElementById("sync-syncnow-state");
     if (broadcaster) {
       if (Weave.Service.clientsEngine.stats.numClients > 1) {
--- a/browser/base/content/test/general/browser_syncui.js
+++ b/browser/base/content/test/general/browser_syncui.js
@@ -174,16 +174,29 @@ function *doTestButtonActivities() {
 }
 
 add_task(function* testButtonActivitiesInNavBar() {
   // check the button's functionality while the button is in the NavBar - which
   // it already is.
   yield doTestButtonActivities();
 });
 
+add_task(function* testFormatLastSyncDateNow() {
+  let now = new Date();
+  let nowString = gSyncUI.formatLastSyncDate(now);
+  Assert.equal(nowString, "Last sync: " + now.toLocaleDateString(undefined, {weekday: 'long', hour: 'numeric', minute: 'numeric'}));
+});
+
+add_task(function* testFormatLastSyncDateMonthAgo() {
+  let monthAgo = new Date();
+  monthAgo.setMonth(monthAgo.getMonth() - 1);
+  let monthAgoString = gSyncUI.formatLastSyncDate(monthAgo);
+  Assert.equal(monthAgoString, "Last sync: " + monthAgo.toLocaleDateString(undefined, {month: 'long', day: 'numeric'}));
+});
+
 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);
   yield PanelUI.show();
   try {
     yield doTestButtonActivities();
   } finally {
--- a/browser/components/syncedtabs/TabListView.js
+++ b/browser/components/syncedtabs/TabListView.js
@@ -46,16 +46,17 @@ function TabListView(window, props) {
   this.tabsFilter = this._doc.querySelector(".tabsFilter");
   this.clearFilter = this._doc.querySelector(".textbox-search-clear");
   this.searchBox = this._doc.querySelector(".search-box");
   this.searchIcon = this._doc.querySelector(".textbox-search-icon");
 
   this.container = this._doc.createElement("div");
 
   this._attachFixedListeners();
+
   this._setupContextMenu();
 }
 
 TabListView.prototype = {
   render(state) {
     // Don't rerender anything; just update attributes, e.g. selection
     if (state.canUpdateAll) {
       this._update(state);
@@ -202,17 +203,19 @@ TabListView.prototype = {
   /**
    * Update the element representing an item, ensuring it's in sync with the
    * underlying data.
    * @param {client} item - Item to use as a source.
    * @param {Element} itemNode - Element to update.
    */
   _updateClient(item, itemNode) {
     itemNode.setAttribute("id", "item-" + item.id);
-    itemNode.setAttribute("title", item.name);
+    let lastSync = new Date(item.lastModified);
+    let lastSyncTitle = getChromeWindow(this._window).gSyncUI.formatLastSyncDate(lastSync);
+    itemNode.setAttribute("title", lastSyncTitle);
     if (item.closed) {
       itemNode.classList.add("closed");
     } else {
       itemNode.classList.remove("closed");
     }
     if (item.selected) {
       itemNode.classList.add("selected");
     } else {
--- a/services/sync/modules/SyncedTabs.jsm
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -79,16 +79,17 @@ let SyncedTabsInternal = {
 
   /* Make a "client" record. Returns a promise for consistency with _makeTab */
   _makeClient: Task.async(function* (client) {
     return {
       id: client.id,
       type: "client",
       name: Weave.Service.clientsEngine.getClientName(client.id),
       isMobile: Weave.Service.clientsEngine.isMobile(client.id),
+      lastModified: client.lastModified * 1000, // sec to ms
       tabs: []
     };
   }),
 
   _tabMatchesFilter(tab, filter) {
     let reFilter = new RegExp(escapeRegExp(filter), "i");
     return tab.url.match(reFilter) || tab.title.match(reFilter);
   },
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -140,17 +140,17 @@ TabStore.prototype = {
     let winEnum = this.getWindowEnumerator();
     while (winEnum.hasMoreElements()) {
       let win = winEnum.getNext();
       if (this.shouldSkipWindow(win)) {
         continue;
       }
 
       for (let tab of win.gBrowser.tabs) {
-        tabState = this.getTabState(tab);
+        let tabState = this.getTabState(tab);
 
         // Make sure there are history entries to look at.
         if (!tabState || !tabState.entries.length) {
           continue;
         }
 
         let acceptable = !filter ? (url) => url :
                                    (url) => url && !filteredUrls.test(url);
@@ -260,17 +260,23 @@ TabStore.prototype = {
   },
 
   wipe: function () {
     this._remoteClients = {};
   },
 
   create: function (record) {
     this._log.debug("Adding remote tabs from " + record.clientName);
-    this._remoteClients[record.id] = record.cleartext;
+    this._remoteClients[record.id] = Object.assign({}, record.cleartext, {
+      lastModified: record.modified
+    });
+
+    // We should remove the following code which sets the notifyTabState pref,
+    // it doesn't seem to be used anywhere and appears broken (we divide by
+    // 1000 but it is already seconds!). See bug 1272806.
 
     // Lose some precision, but that's good enough (seconds).
     let roundModify = Math.floor(record.modified / 1000);
     let notifyState = Svc.Prefs.get("notifyTabState");
 
     // If there's no existing pref, save this first modified time.
     if (notifyState == null) {
       Svc.Prefs.set("notifyTabState", roundModify);
--- a/services/sync/tests/unit/test_tab_store.js
+++ b/services/sync/tests/unit/test_tab_store.js
@@ -15,108 +15,108 @@ function getMockStore() {
 }
 
 function test_create() {
   let store = new TabEngine(Service)._store;
 
   _("Create a first record");
   let rec = {id: "id1",
              clientName: "clientName1",
-             cleartext: "cleartext1",
+             cleartext: { "foo": "bar" },
              modified: 1000};
   store.applyIncoming(rec);
-  do_check_eq(store._remoteClients["id1"], "cleartext1");
-  do_check_eq(Svc.Prefs.get("notifyTabState"), 1);
+  deepEqual(store._remoteClients["id1"], { lastModified: 1000, foo: "bar" });
+  equal(Svc.Prefs.get("notifyTabState"), 1);
 
   _("Create a second record");
   rec = {id: "id2",
          clientName: "clientName2",
-         cleartext: "cleartext2",
+         cleartext: { "foo2": "bar2" },
          modified: 2000};
   store.applyIncoming(rec);
-  do_check_eq(store._remoteClients["id2"], "cleartext2");
-  do_check_eq(Svc.Prefs.get("notifyTabState"), 0);
+  deepEqual(store._remoteClients["id2"], { lastModified: 2000, foo2: "bar2" });
+  equal(Svc.Prefs.get("notifyTabState"), 0);
 
   _("Create a third record");
   rec = {id: "id3",
          clientName: "clientName3",
-         cleartext: "cleartext3",
+         cleartext: { "foo3": "bar3" },
          modified: 3000};
   store.applyIncoming(rec);
-  do_check_eq(store._remoteClients["id3"], "cleartext3");
-  do_check_eq(Svc.Prefs.get("notifyTabState"), 0);
+  deepEqual(store._remoteClients["id3"], { lastModified: 3000, foo3: "bar3" });
+  equal(Svc.Prefs.get("notifyTabState"), 0);
 
   // reset the notifyTabState
   Svc.Prefs.reset("notifyTabState");
 }
 
 function test_getAllTabs() {
   let store = getMockStore();
   let tabs;
 
   let threeUrls = ["http://foo.com", "http://fuubar.com", "http://barbar.com"];
 
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://bar.com", 1, 1, () => 2, () => threeUrls);
 
   _("Get all tabs.");
   tabs = store.getAllTabs();
   _("Tabs: " + JSON.stringify(tabs));
-  do_check_eq(tabs.length, 1);
-  do_check_eq(tabs[0].title, "title");
-  do_check_eq(tabs[0].urlHistory.length, 2);
-  do_check_eq(tabs[0].urlHistory[0], "http://foo.com");
-  do_check_eq(tabs[0].urlHistory[1], "http://bar.com");
-  do_check_eq(tabs[0].icon, "image");
-  do_check_eq(tabs[0].lastUsed, 1);
+  equal(tabs.length, 1);
+  equal(tabs[0].title, "title");
+  equal(tabs[0].urlHistory.length, 2);
+  equal(tabs[0].urlHistory[0], "http://foo.com");
+  equal(tabs[0].urlHistory[1], "http://bar.com");
+  equal(tabs[0].icon, "image");
+  equal(tabs[0].lastUsed, 1);
 
   _("Get all tabs, and check that filtering works.");
   let twoUrls = ["about:foo", "http://fuubar.com"];
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1, () => 2, () => twoUrls);
   tabs = store.getAllTabs(true);
   _("Filtered: " + JSON.stringify(tabs));
-  do_check_eq(tabs.length, 0);
+  equal(tabs.length, 0);
 
   _("Get all tabs, and check that the entries safety limit works.");
   let allURLs = [];
   for (let i = 0; i < 50; i++) {
     allURLs.push("http://foo" + i + ".bar");
   }
   allURLs.splice(35, 0, "about:foo", "about:bar", "about:foobar");
 
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://bar.com", 1, 1, () => 45, () => allURLs);
   tabs = store.getAllTabs((url) => url.startsWith("about"));
 
   _("Sliced: " + JSON.stringify(tabs));
-  do_check_eq(tabs.length, 1);
-  do_check_eq(tabs[0].urlHistory.length, 25);
-  do_check_eq(tabs[0].urlHistory[0], "http://foo40.bar");
-  do_check_eq(tabs[0].urlHistory[24], "http://foo16.bar");
+  equal(tabs.length, 1);
+  equal(tabs[0].urlHistory.length, 25);
+  equal(tabs[0].urlHistory[0], "http://foo40.bar");
+  equal(tabs[0].urlHistory[24], "http://foo16.bar");
 }
 
 function test_createRecord() {
   let store = getMockStore();
   let record;
 
   store.getTabState = mockGetTabState;
   store.shouldSkipWindow = mockShouldSkipWindow;
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1);
 
   let tabs = store.getAllTabs();
   let tabsize = JSON.stringify(tabs[0]).length;
   let numtabs = Math.ceil(20000./77.);
 
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1);
   record = store.createRecord("fake-guid");
-  do_check_true(record instanceof TabSetRecord);
-  do_check_eq(record.tabs.length, 1);
+  ok(record instanceof TabSetRecord);
+  equal(record.tabs.length, 1);
 
   _("create a big record");
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, numtabs);
   record = store.createRecord("fake-guid");
-  do_check_true(record instanceof TabSetRecord);
-  do_check_eq(record.tabs.length, 256);
+  ok(record instanceof TabSetRecord);
+  equal(record.tabs.length, 256);
 }
 
 function run_test() {
   test_create();
   test_getAllTabs();
   test_createRecord();
 }