Bug 1244622 - Add a Show All (tabs) button in the Synced Tabs menu. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Mon, 06 Feb 2017 16:31:31 -0500
changeset 480061 abb34890f2b6e2edee8383b49e64885eb10b2cd6
parent 479958 e677ba018b22558fef1d07b74d416fd3a28a5dc3
child 544868 7cd95610fff0a75a600d37f978ac6ba9e849bd36
push id44450
push userbmo:eoger@fastmail.com
push dateTue, 07 Feb 2017 20:45:37 +0000
reviewersmarkh
bugs1244622
milestone54.0a1
Bug 1244622 - Add a Show All (tabs) button in the Synced Tabs menu. r?markh MozReview-Commit-ID: 7z9fTVCDGgF
browser/components/customizableui/CustomizableWidgets.jsm
browser/components/customizableui/content/panelUI.inc.xul
browser/locales/en-US/chrome/browser/browser.dtd
browser/themes/shared/customizableui/panelUI.inc.css
services/sync/modules/SyncedTabs.jsm
--- a/browser/components/customizableui/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/CustomizableWidgets.jsm
@@ -304,16 +304,18 @@ const CustomizableWidgets = [
     viewId: "PanelUI-remotetabs",
     defaultArea: CustomizableUI.AREA_PANEL,
     deckIndices: {
       DECKINDEX_TABS: 0,
       DECKINDEX_TABSDISABLED: 1,
       DECKINDEX_FETCHING: 2,
       DECKINDEX_NOCLIENTS: 3,
     },
+    MAX_TABS_PER_CLIENT: 20,
+    SHOW_ALL_TABS_MIN_TABS: 5, // Minimum number of tabs displayed when we click "Show All"
     onCreated(aNode) {
       // Add an observer to the button so we get the animation during sync.
       // (Note the observer sets many attributes, including label and
       // tooltiptext, but we only want the 'syncstatus' attribute for the
       // animation)
       let doc = aNode.ownerDocument;
       let obnode = doc.createElementNS(kNSXUL, "observes");
       obnode.setAttribute("element", "sync-status");
@@ -396,23 +398,23 @@ const CustomizableWidgets = [
       // We call setAttribute instead of relying on the XBL property setter due
       // to things going wrong when we try and set the index before the XBL
       // binding has been created - see bug 1241851 for the gory details.
       deck.setAttribute("selectedIndex", index);
     },
 
     _showTabsPromise: Promise.resolve(),
     // Update the tab list after any existing in-flight updates are complete.
-    _showTabs() {
+    _showTabs(showMoreFromClientId) {
       this._showTabsPromise = this._showTabsPromise.then(() => {
-        return this.__showTabs();
+        return this.__showTabs(showMoreFromClientId);
       });
     },
     // Return a new promise to update the tab list.
-    __showTabs() {
+    __showTabs(showMoreFromClientId) {
       let doc = this._tabsList.ownerDocument;
       return SyncedTabs.getTabClients().then(clients => {
         // The view may have been hidden while the promise was resolving.
         if (!this._tabsList) {
           return;
         }
         if (clients.length === 0 && !SyncedTabs.hasSyncedThisSession) {
           // the "fetching tabs" deck is being shown - let's leave it there.
@@ -422,26 +424,26 @@ const CustomizableWidgets = [
 
         if (clients.length === 0) {
           this.setDeckIndex(this.deckIndices.DECKINDEX_NOCLIENTS);
           return;
         }
 
         this.setDeckIndex(this.deckIndices.DECKINDEX_TABS);
         this._clearTabList();
-        SyncedTabs.sortTabClientsByLastUsed(clients, 50 /* maxTabs */);
+        SyncedTabs.sortTabClientsByLastUsed(clients, 50 /* maxTabs */, showMoreFromClientId);
         let fragment = doc.createDocumentFragment();
 
         for (let client of clients) {
           // add a menu separator for all clients other than the first.
           if (fragment.lastChild) {
             let separator = doc.createElementNS(kNSXUL, "menuseparator");
             fragment.appendChild(separator);
           }
-          this._appendClient(client, fragment);
+          this._appendClient(client, fragment, showMoreFromClientId == client.id);
         }
         this._tabsList.appendChild(fragment);
       }).catch(err => {
         Cu.reportError(err);
       }).then(() => {
         // an observer for tests.
         Services.obs.notifyObservers(null, "synced-tabs-menu:test:tabs-updated", null);
       });
@@ -461,36 +463,49 @@ const CustomizableWidgets = [
       }
       let message = this._tabsList.getAttribute(messageAttr);
       let doc = this._tabsList.ownerDocument;
       let messageLabel = doc.createElementNS(kNSXUL, "label");
       messageLabel.textContent = message;
       appendTo.appendChild(messageLabel);
       return messageLabel;
     },
-    _appendClient(client, attachFragment) {
+    _appendClient(client, attachFragment, ignoreTabsLimit) {
       let doc = attachFragment.ownerDocument;
       // Create the element for the remote client.
       let clientItem = doc.createElementNS(kNSXUL, "label");
       clientItem.setAttribute("itemtype", "client");
       let window = doc.defaultView;
       clientItem.setAttribute("tooltiptext",
         window.gSyncUI.formatLastSyncDate(new Date(client.lastModified)));
       clientItem.textContent = client.name;
 
       attachFragment.appendChild(clientItem);
 
       if (client.tabs.length == 0) {
         let label = this._appendMessageLabel("notabsforclientlabel", attachFragment);
         label.setAttribute("class", "PanelUI-remotetabs-notabsforclient-label");
       } else {
+        let displayShowAllTabs = !ignoreTabsLimit && client.tabs.length > this.MAX_TABS_PER_CLIENT;
+        if (displayShowAllTabs) {
+          // When the user clicks "Show All", try to have at least SHOW_ALL_TABS_MIN_TABS more tabs
+          // to display in order to avoid user frustration
+          let sliceEnd = ((client.tabs.length - this.SHOW_ALL_TABS_MIN_TABS) < this.MAX_TABS_PER_CLIENT) ?
+                         client.tabs.length - this.SHOW_ALL_TABS_MIN_TABS :
+                         this.MAX_TABS_PER_CLIENT;
+          client.tabs = client.tabs.slice(0, sliceEnd);
+        }
         for (let tab of client.tabs) {
           let tabEnt = this._createTabElement(doc, tab);
           attachFragment.appendChild(tabEnt);
         }
+        if (displayShowAllTabs) {
+          let showAllEnt = this._createShowAllElement(doc, client.id);
+          attachFragment.appendChild(showAllEnt);
+        }
       }
     },
     _createTabElement(doc, tabInfo) {
       let item = doc.createElementNS(kNSXUL, "toolbarbutton");
       let tooltipText = (tabInfo.title ? tabInfo.title + "\n" : "") + tabInfo.url;
       item.setAttribute("itemtype", "tab");
       item.setAttribute("class", "subviewbutton");
       item.setAttribute("targetURI", tabInfo.url);
@@ -501,16 +516,31 @@ const CustomizableWidgets = [
       // respects different buttons (eg, to open in a new tab).
       item.addEventListener("click", e => {
         doc.defaultView.openUILink(tabInfo.url, e);
         CustomizableUI.hidePanelForNode(item);
         BrowserUITelemetry.countSyncedTabEvent("open", "toolbarbutton-subview");
       });
       return item;
     },
+    _createShowAllElement(doc, clientId) {
+      let showAllItem = doc.createElementNS(kNSXUL, "toolbarbutton");
+      showAllItem.setAttribute("itemtype", "showallbutton");
+      showAllItem.setAttribute("class", "subviewbutton");
+      let label = this._tabsList.getAttribute("showAllLabel");
+      showAllItem.setAttribute("label", label);
+      let tooltipText = this._tabsList.getAttribute("showAllTooltipText");
+      showAllItem.setAttribute("tooltiptext", tooltipText);
+      showAllItem.addEventListener("click", e => {
+        e.preventDefault();
+        e.stopPropagation();
+        this._showTabs(clientId);
+      });
+      return showAllItem;
+    }
   }, {
     id: "privatebrowsing-button",
     shortcutId: "key_privatebrowsing",
     defaultArea: CustomizableUI.AREA_PANEL,
     onCommand(e) {
       let win = e.target.ownerGlobal;
       win.OpenBrowserWindow({private: true});
     }
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -120,16 +120,18 @@
                            oncommand="gSyncUI.doSync();"
                            closemenu="none"/>
             <menuseparator id="PanelUI-remotetabs-separator"/>
           </vbox>
           <deck id="PanelUI-remotetabs-deck">
             <!-- Sync is ready to Sync and the "tabs" engine is enabled -->
             <vbox id="PanelUI-remotetabs-tabspane">
               <vbox id="PanelUI-remotetabs-tabslist"
+                    showAllLabel="&appMenuRemoteTabs.showAll.label;"
+                    showAllTooltipText="&appMenuRemoteTabs.showAll.tooltip;"
                     notabsforclientlabel="&appMenuRemoteTabs.notabs.label;"
                     />
             </vbox>
             <!-- Sync is ready to Sync but the "tabs" engine isn't enabled-->
             <hbox id="PanelUI-remotetabs-tabsdisabledpane" pack="center" flex="1">
               <vbox class="PanelUI-remotetabs-instruction-box">
                 <hbox pack="center">
                   <image class="fxaSyncIllustration" alt=""/>
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -353,16 +353,20 @@ These should match what Safari and other
 <!ENTITY appMenuHistory.restoreSession.label "Restore Previous Session">
 <!ENTITY appMenuHistory.viewSidebar.label "View History Sidebar">
 <!ENTITY appMenuHelp.tooltip "Open Help Menu">
 
 <!ENTITY appMenuRemoteTabs.label "Synced Tabs">
 <!-- LOCALIZATION NOTE (appMenuRemoteTabs.notabs.label): This is shown beneath
      the name of a device when that device has no open tabs -->
 <!ENTITY appMenuRemoteTabs.notabs.label "No open tabs">
+<!-- LOCALIZATION NOTE (appMenuRemoteTabs.showAll.label, appMenuRemoteTabs.showAll.tooltip):
+     This is shown after the tabs list if we can display more tabs by clicking on the button -->
+<!ENTITY appMenuRemoteTabs.showAll.label "Show All">
+<!ENTITY appMenuRemoteTabs.showAll.tooltip "Show all tabs from this device">
 <!-- LOCALIZATION NOTE (appMenuRemoteTabs.tabsnotsyncing.label): This is shown
      when Sync is configured but syncing tabs is disabled. -->
 <!ENTITY appMenuRemoteTabs.tabsnotsyncing.label "Turn on tab syncing to view a list of tabs from your other devices.">
 <!-- LOCALIZATION NOTE (appMenuRemoteTabs.noclients.label): This is shown
      when Sync is configured but this appears to be the only device attached to
      the account. We also show links to download Firefox for android/ios. -->
 <!ENTITY appMenuRemoteTabs.noclients.title "No synced tabs… yet!">
 <!ENTITY appMenuRemoteTabs.noclients.subtitle "Want to see your tabs from other devices here?">
--- a/browser/themes/shared/customizableui/panelUI.inc.css
+++ b/browser/themes/shared/customizableui/panelUI.inc.css
@@ -1239,29 +1239,29 @@ menuitem.panel-subview-footer@menuStateA
 
 .subviewbutton > .menu-accel-container {
   -moz-box-pack: start;
   margin-inline-start: 10px;
   margin-inline-end: auto;
   color: GrayText;
 }
 
-#PanelUI-remotetabs-tabslist > toolbarbutton,
+#PanelUI-remotetabs-tabslist > toolbarbutton[itemtype="tab"],
 #PanelUI-historyItems > toolbarbutton {
   list-style-image: url("chrome://mozapps/skin/places/defaultFavicon.png");
 }
 
 @media (min-resolution: 1.1dppx) {
-  #PanelUI-remotetabs-tabslist > toolbarbutton,
+  #PanelUI-remotetabs-tabslist > toolbarbutton[itemtype="tab"],
   #PanelUI-historyItems > toolbarbutton {
     list-style-image: url("chrome://mozapps/skin/places/defaultFavicon@2x.png");
   }
 }
 
-#PanelUI-remotetabs-tabslist > toolbarbutton > .toolbarbutton-icon,
+#PanelUI-remotetabs-tabslist > toolbarbutton[itemtype="tab"] > .toolbarbutton-icon,
 #PanelUI-recentlyClosedWindows > toolbarbutton > .toolbarbutton-icon,
 #PanelUI-recentlyClosedTabs > toolbarbutton > .toolbarbutton-icon,
 #PanelUI-historyItems > toolbarbutton > .toolbarbutton-icon {
   width: 16px;
   height: 16px;
 }
 
 toolbarbutton[panel-multiview-anchor="true"],
--- a/services/sync/modules/SyncedTabs.jsm
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -267,26 +267,23 @@ this.SyncedTabs = {
   // resolves when the sync is complete, but there's no resolved value -
   // callers should be listening for TOPIC_TABS_CHANGED.
   // If |force| is true we always sync. If false, we only sync if the most
   // recent sync wasn't "recently".
   syncTabs(force) {
     return this._internal.syncTabs(force);
   },
 
-  sortTabClientsByLastUsed(clients, maxTabs = Infinity) {
-    // First sort and filter the list of tabs for each client. Note that
+  sortTabClientsByLastUsed(clients) {
+    // First sort the list of tabs for each client. Note that
     // this module promises that the objects it returns are never
     // shared, so we are free to mutate those objects directly.
     for (let client of clients) {
       let tabs = client.tabs;
       tabs.sort((a, b) => b.lastUsed - a.lastUsed);
-      if (Number.isFinite(maxTabs)) {
-        client.tabs = tabs.slice(0, maxTabs);
-      }
     }
     // Now sort the clients - the clients are sorted in the order of the
     // most recent tab for that client (ie, it is important the tabs for
     // each client are already sorted.)
     clients.sort((a, b) => {
       if (a.tabs.length == 0) {
         return 1; // b comes first.
       }