Bug 1377968 - Add a tools section and subview to the Bookmarks subview inside the Library widget. r=Gijs
authorMike de Boer <mdeboer@mozilla.com>
Tue, 15 Aug 2017 13:34:41 +0200
changeset 374804 70729ad46076f5ed89eb6d9d2c3287c45a232c6e
parent 374803 aefa7575ba78021e70fd9c7e83f0808abe03743a
child 374805 c130d346bc41feb274dbfa172568404f932ce6d0
push id32339
push userkwierso@gmail.com
push dateWed, 16 Aug 2017 01:51:50 +0000
treeherdermozilla-central@6966f27380bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1377968
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1377968 - Add a tools section and subview to the Bookmarks subview inside the Library widget. r=Gijs MozReview-Commit-ID: GqbUMbuvYoO
browser/base/content/browser-places.js
browser/base/content/browser-sidebar.js
browser/base/content/browser.js
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/panelUI.inc.xul
browser/locales/en-US/chrome/browser/browser.dtd
browser/themes/shared/icons/toolbar.svg
browser/themes/shared/jar.inc.mn
browser/themes/shared/menupanel.inc.css
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1939,16 +1939,79 @@ var BookmarkingUI = {
       return;
     if (PlacesUtils.nodeIsContainer(target._placesNode))
       PlacesCommandHook.showPlacesOrganizer([ "BookmarksMenu", target._placesNode.itemId ]);
     else
       PlacesUIUtils.openNodeWithEvent(target._placesNode, aEvent);
     PanelUI.hide();
   },
 
+  showBookmarkingTools(triggerNode) {
+    const panelID = "PanelUI-bookmarkingTools";
+    let viewNode = document.getElementById(panelID);
+    for (let button of [...viewNode.getElementsByTagName("toolbarbutton")]) {
+      let update = true;
+      switch (button.id) {
+        case "panelMenu_toggleBookmarksMenu":
+          let placement = CustomizableUI.getPlacementOfWidget(this.BOOKMARK_BUTTON_ID);
+          button.setAttribute("checked", !!placement && placement.area == CustomizableUI.AREA_NAVBAR);
+          break;
+        case "panelMenu_viewBookmarksSidebar":
+          button.setAttribute("checked", SidebarUI.currentID == "viewBookmarksSidebar");
+          break;
+        default:
+          update = false;
+          break;
+      }
+      if (update) {
+        updateToggleControlLabel(button);
+      }
+    }
+    PanelUI.showSubView(panelID, triggerNode);
+  },
+
+  toggleMenuButtonInToolbar(triggerNode) {
+    let placement = CustomizableUI.getPlacementOfWidget(this.BOOKMARK_BUTTON_ID);
+    const area = CustomizableUI.AREA_NAVBAR;
+    if (!placement) {
+      // Button is in the palette, so we can move it to the navbar.
+      let pos;
+      let widgetIDs = CustomizableUI.getWidgetIdsInArea(CustomizableUI.AREA_NAVBAR);
+      // If there's a spring inside the navbar, find it and use that as the
+      // placement marker.
+      let lastSpringID = null;
+      for (let i = widgetIDs.length - 1; i >= 0; --i) {
+        let id = widgetIDs[i];
+        if (CustomizableUI.isSpecialWidget(id) && /spring/.test(id)) {
+          lastSpringID = id;
+          break;
+        }
+      }
+      if (lastSpringID) {
+        pos = CustomizableUI.getPlacementOfWidget(lastSpringID).position + 1;
+      } else {
+        // Next alternative is to use the searchbar as the placement marker.
+        const searchWidgetID = "search-container";
+        if (widgetIDs.includes(searchWidgetID)) {
+          pos = CustomizableUI.getPlacementOfWidget(searchWidgetID).position + 1;
+        } else {
+          // Last alternative is to use the navbar as the placement marker.
+          pos = CustomizableUI.getPlacementOfWidget("urlbar-container").position + 1;
+        }
+      }
+
+      CustomizableUI.addWidgetToArea(this.BOOKMARK_BUTTON_ID, area, pos);
+    } else {
+      // Move it back to the palette.
+      CustomizableUI.removeWidgetFromArea(this.BOOKMARK_BUTTON_ID);
+    }
+    triggerNode.setAttribute("checked", !placement);
+    updateToggleControlLabel(triggerNode);
+  },
+
   // nsINavBookmarkObserver
   onItemAdded(aItemId, aParentId, aIndex, aItemType, aURI, aTitle, aDateAdded, aGuid) {
     if (aURI && aURI.equals(this._uri)) {
       // If a new bookmark has been added to the tracked uri, register it.
       if (!this._itemGuids.has(aGuid)) {
         this._itemGuids.add(aGuid);
         // Only need to update the UI if it wasn't marked as starred before:
         if (this._itemGuids.size == 1) {
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -283,44 +283,51 @@ var SidebarUI = {
     return null;
   },
 
   /**
    * Toggle the visibility of the sidebar. If the sidebar is hidden or is open
    * with a different commandID, then the sidebar will be opened using the
    * specified commandID. Otherwise the sidebar will be hidden.
    *
-   * @param {string} commandID ID of the xul:broadcaster element to use.
+   * @param  {string}  commandID     ID of the xul:broadcaster element to use.
+   * @param  {DOMNode} [triggerNode] Node, usually a button, that triggered the
+   *                                 visibility toggling of the sidebar.
    * @return {Promise}
    */
-  toggle(commandID = this.lastOpenedId) {
+  toggle(commandID = this.lastOpenedId, triggerNode) {
     // First priority for a default value is this.lastOpenedId which is set during show()
     // and not reset in hide(), unlike currentID. If show() hasn't been called or the command
     // doesn't exist anymore, then fallback to a default sidebar.
     if (!commandID || !this.getBroadcasterById(commandID)) {
       commandID = this.DEFAULT_SIDEBAR_ID;
     }
 
     if (this.isOpen && commandID == this.currentID) {
-      this.hide();
+      this.hide(triggerNode);
       return Promise.resolve();
     }
-    return this.show(commandID);
+    return this.show(commandID, triggerNode);
   },
 
   /**
    * Show the sidebar, using the parameters from the specified broadcaster.
    * @see SidebarUI note.
    *
    * This wraps the internal method, including a ping to telemetry.
    *
-   * @param {string} commandID ID of the xul:broadcaster element to use.
+   * @param {string}  commandID     ID of the xul:broadcaster element to use.
+   * @param {DOMNode} [triggerNode] Node, usually a button, that triggered the
+   *                                showing of the sidebar.
    */
-  show(commandID) {
+  show(commandID, triggerNode) {
     return this._show(commandID).then(() => {
+      if (triggerNode) {
+        updateToggleControlLabel(triggerNode);
+      }
       BrowserUITelemetry.countSidebarEvent(commandID, "show");
     });
   },
 
   /**
    * Implementation for show. Also used internally for sidebars that are shown
    * when a window is opened and we don't want to ping telemetry.
    *
@@ -398,18 +405,21 @@ var SidebarUI = {
       selBrowser.messageManager.sendAsyncMessage("Sidebar:VisibilityChange",
         {commandID, isOpen: true}
       );
     });
   },
 
   /**
    * Hide the sidebar.
+   *
+   * @param {DOMNode} [triggerNode] Node, usually a button, that triggered the
+   *                                hiding of the sidebar.
    */
-  hide() {
+  hide(triggerNode) {
     if (!this.isOpen) {
       return;
     }
 
     this.hideSwitcherPanel();
 
     let commandID = this._box.getAttribute("sidebarcommand");
     let sidebarBroadcaster = document.getElementById(commandID);
@@ -432,16 +442,19 @@ var SidebarUI = {
     this.title = "";
     this._box.hidden = this._splitter.hidden = true;
 
     let selBrowser = gBrowser.selectedBrowser;
     selBrowser.focus();
     selBrowser.messageManager.sendAsyncMessage("Sidebar:VisibilityChange",
       {commandID, isOpen: false}
     );
+    if (triggerNode) {
+      updateToggleControlLabel(triggerNode);
+    }
     BrowserUITelemetry.countSidebarEvent(commandID, "hide");
   },
 };
 
 // Add getters related to the position here, since we will want them
 // available for both startDelayedLoad and init.
 XPCOMUtils.defineLazyPreferenceGetter(SidebarUI, "_positionStart",
   SidebarUI.POSITION_START_PREF, true, SidebarUI.setPosition.bind(SidebarUI));
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5476,19 +5476,21 @@ function onViewToolbarsPopupShowing(aEve
     removeFromToolbar.removeAttribute("disabled");
   } else {
     moveToPanel.setAttribute("disabled", true);
     removeFromToolbar.setAttribute("disabled", true);
   }
 }
 
 function onViewToolbarCommand(aEvent) {
-  var toolbarId = aEvent.originalTarget.getAttribute("toolbarId");
-  var isVisible = aEvent.originalTarget.getAttribute("checked") == "true";
+  let node = aEvent.originalTarget;
+  let toolbarId = node.getAttribute("toolbarId");
+  let isVisible = node.getAttribute("checked") == "true";
   CustomizableUI.setToolbarVisibility(toolbarId, isVisible);
+  updateToggleControlLabel(node);
 }
 
 function setToolbarVisibility(toolbar, isVisible, persist = true) {
   let hidingAttribute;
   if (toolbar.getAttribute("type") == "menubar") {
     hidingAttribute = "autohide";
     if (AppConstants.platform == "linux") {
       Services.prefs.setBoolPref("ui.key.menuAccessKeyFocuses", !isVisible);
@@ -5510,16 +5512,28 @@ function setToolbarVisibility(toolbar, i
   };
   let event = new CustomEvent("toolbarvisibilitychange", eventParams);
   toolbar.dispatchEvent(event);
 
   PlacesToolbarHelper.init();
   BookmarkingUI.onToolbarVisibilityChange();
 }
 
+function updateToggleControlLabel(control) {
+  if (!control.hasAttribute("label-checked")) {
+    return;
+  }
+
+  if (!control.hasAttribute("label-unchecked")) {
+    control.setAttribute("label-unchecked", control.getAttribute("label"));
+  }
+  let prefix = (control.getAttribute("checked") == "true") ? "" : "un";
+  control.setAttribute("label", control.getAttribute(`label-${prefix}checked`));
+}
+
 var TabletModeUpdater = {
   init() {
     if (AppConstants.isPlatformAndVersionAtLeast("win", "10")) {
       this.update(WindowsUIUtils.inTabletMode);
       Services.obs.addObserver(this, "tablet-mode-change");
     }
   },
 
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -698,16 +698,22 @@ this.PanelMultiView = class {
       if (playTransition || this.panelViews) {
         previousRect = this._cachePreviousRectAndConstrainBounds(viewNode, previousViewNode);
       }
 
       if (!await this._dispatchViewShowing(viewNode, aAnchor)) {
         return;
       }
 
+      // If this instance was destructed in the meantime, there's no point in
+      // trying to show anything here.
+      if (!this.node) {
+        return;
+      }
+
       this._currentSubView = viewNode;
       viewNode.setAttribute("current", true);
 
       if (this.panelViews) {
         this.node.setAttribute("viewtype", "subview");
         if (!playTransition) {
           this.descriptionHeightWorkaround(viewNode);
         } else {
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -229,23 +229,21 @@
     <panelview id="PanelUI-bookmarks" flex="1" class="PanelUI-subView">
       <label value="&bookmarksMenu.label;" class="panel-subview-header"/>
       <vbox class="panel-subview-body">
         <toolbarbutton id="panelMenuBookmarkThisPage"
                        class="subviewbutton subviewbutton-iconic"
                        observes="bookmarkThisPageBroadcaster"
                        command="Browser:AddBookmarkAs"
                        onclick="PanelUI.hide();"/>
-        <toolbarbutton id="panelMenu_viewBookmarksSidebar"
-                       label="&viewBookmarksSidebar2.label;"
-                       class="subviewbutton subviewbutton-iconic"
-                       key="viewBookmarksSidebarKb"
-                       oncommand="SidebarUI.toggle('viewBookmarksSidebar'); PanelUI.hide();">
-          <observes element="viewBookmarksSidebar" attribute="checked"/>
-        </toolbarbutton>
+        <toolbarbutton id="panelMenu_bookmarkingTools"
+                       label="&bookmarkingTools.label;"
+                       class="subviewbutton subviewbutton-iconic subviewbutton-nav"
+                       closemenu="none"
+                       oncommand="BookmarkingUI.showBookmarkingTools(this);"/>
         <toolbarbutton id="panelMenu_searchBookmarks"
                        label="&searchBookmarks.label;"
                        class="subviewbutton subviewbutton-iconic"
                        oncommand="PlacesCommandHook.searchBookmarks(); PanelUI.hide();"/>
         <toolbarseparator/>
         <label id="panelMenu_recentBookmarks"
                value="&recentBookmarks.label;"
                class="subview-subheader"/>
@@ -731,10 +729,36 @@
                        oncommand="PanelUI.showSubView('PanelUI-history', this)"/>
         <toolbarbutton id="appMenu-library-remotetabs-button"
                        class="subviewbutton subviewbutton-iconic subviewbutton-nav"
                        label="&appMenuRemoteTabs.label;"
                        closemenu="none"
                        oncommand="PanelUI.showSubView('PanelUI-remotetabs', this)"/>
       </vbox>
     </panelview>
+
+    <panelview id="PanelUI-bookmarkingTools" class="PanelUI-subView">
+      <vbox class="panel-subview-body">
+        <toolbarbutton id="panelMenu_toggleBookmarksMenu"
+                       label="&addBookmarksMenu.label;"
+                       label-checked="&removeBookmarksMenu.label;"
+                       class="subviewbutton subviewbutton-iconic"
+                       oncommand="BookmarkingUI.toggleMenuButtonInToolbar(this); PanelUI.hide();"/>
+        <toolbarbutton id="panelMenu_viewBookmarksSidebar"
+                       label="&viewBookmarksSidebar2.label;"
+                       label-checked="&hideBookmarksSidebar.label;"
+                       class="subviewbutton subviewbutton-iconic"
+                       key="viewBookmarksSidebarKb"
+                       oncommand="SidebarUI.toggle('viewBookmarksSidebar', this); PanelUI.hide();">
+          <observes element="viewBookmarksSidebar" attribute="checked"/>
+        </toolbarbutton>
+        <toolbarbutton id="panelMenu_viewBookmarksToolbar"
+                       class="subviewbutton subviewbutton-iconic"
+                       placesanonid="view-toolbar"
+                       toolbarId="PersonalToolbar"
+                       type="checkbox"
+                       oncommand="onViewToolbarCommand(event)"
+                       label="&viewBookmarksToolbar.label;"
+                       label-checked="&hideBookmarksToolbar.label;"/>
+      </vbox>
+    </panelview>
   </photonpanelmultiview>
 </panel>
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -239,21 +239,26 @@ These should match what Safari and other
 
 <!-- Toolbar items -->
 <!ENTITY homeButton.label             "Home">
 
 <!ENTITY bookmarksButton.label          "Bookmarks">
 <!ENTITY bookmarksCmd.commandkey "b">
 
 <!ENTITY bookmarksMenuButton.label          "Bookmarks">
-<!ENTITY bookmarksMenuButton.other.label "Other Bookmarks">
-<!ENTITY bookmarksMenuButton.mobile.label "Mobile Bookmarks">
+<!ENTITY bookmarksMenuButton.other.label    "Other Bookmarks">
+<!ENTITY bookmarksMenuButton.mobile.label   "Mobile Bookmarks">
 <!ENTITY viewBookmarksSidebar2.label        "View Bookmarks Sidebar">
+<!ENTITY hideBookmarksSidebar.label         "Hide Bookmarks Sidebar">
 <!ENTITY viewBookmarksToolbar.label         "View Bookmarks Toolbar">
+<!ENTITY hideBookmarksToolbar.label         "Hide Bookmarks Toolbar">
 <!ENTITY searchBookmarks.label              "Search Bookmarks">
+<!ENTITY bookmarkingTools.label             "Bookmarking Tools">
+<!ENTITY addBookmarksMenu.label             "Add Bookmarks Menu to Toolbar">
+<!ENTITY removeBookmarksMenu.label          "Remove Bookmarks Menu from Toolbar">
 
 <!ENTITY containersMenu.label "Containers">
 
 <!-- LOCALIZATION NOTE (bookmarksSidebarGtkCmd.commandkey): This command
   -  key should not contain the letters A-F, since these are reserved
   -  shortcut keys on Linux. -->
 <!ENTITY bookmarksGtkCmd.commandkey "o">
 <!ENTITY bookmarksWinCmd.commandkey "i">
new file mode 100644
--- /dev/null
+++ b/browser/themes/shared/icons/toolbar.svg
@@ -0,0 +1,6 @@
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
+  <path fill="context-fill" d="M13 1H3a3.007 3.007 0 0 0-3 3v8a3.009 3.009 0 0 0 3 3h10a3.005 3.005 0 0 0 3-3V4a3.012 3.012 0 0 0-3-3zM3 3h10a1 1 0 0 1 1 1v1H2V4a1 1 0 0 1 1-1zm11 3v1H2V6zm-1 7H3a1 1 0 0 1-1-1V8h12v4a1 1 0 0 1-1 1z"/>
+</svg>
--- a/browser/themes/shared/jar.inc.mn
+++ b/browser/themes/shared/jar.inc.mn
@@ -152,16 +152,17 @@
   skin/classic/browser/settings.svg                   (../shared/icons/settings.svg)
   skin/classic/browser/share.svg                      (../shared/icons/share.svg)
   skin/classic/browser/sidebars.svg                   (../shared/icons/sidebars.svg)
   skin/classic/browser/sidebars-right.svg             (../shared/icons/sidebars-right.svg)
   skin/classic/browser/stop.svg                       (../shared/icons/stop.svg)
   skin/classic/browser/stop-to-reload.svg             (../shared/icons/stop-to-reload.svg)
   skin/classic/browser/sync.svg                       (../shared/icons/sync.svg)
   skin/classic/browser/synced-tabs.svg                (../shared/icons/synced-tabs.svg)
+  skin/classic/browser/toolbar.svg                    (../shared/icons/toolbar.svg)
   skin/classic/browser/webIDE.svg                     (../shared/icons/webIDE.svg)
   skin/classic/browser/zoom-in.svg                    (../shared/icons/zoom-in.svg)
   skin/classic/browser/zoom-out.svg                   (../shared/icons/zoom-out.svg)
 
 
   skin/classic/browser/search-indicator.png                    (../shared/search/search-indicator.png)
   skin/classic/browser/search-indicator@2x.png                 (../shared/search/search-indicator@2x.png)
   skin/classic/browser/search-engine-placeholder.png           (../shared/search/search-engine-placeholder.png)
--- a/browser/themes/shared/menupanel.inc.css
+++ b/browser/themes/shared/menupanel.inc.css
@@ -93,27 +93,36 @@
 }
 
 #appMenuViewHistorySidebar,
 #PanelUI-remotetabs-view-sidebar,
 #panelMenu_viewBookmarksSidebar {
   list-style-image: url("chrome://browser/skin/sidebars.svg");
 }
 
+#panelMenu_bookmarkingTools {
+  list-style-image: url("chrome://browser/skin/developer.svg");
+}
+
+#panelMenu_viewBookmarksToolbar {
+  list-style-image: url("chrome://browser/skin/toolbar.svg");
+}
+
 #appMenu-library-bookmarks-button,
 #panelMenuBookmarkThisPage {
   list-style-image: url("chrome://browser/skin/bookmark-hollow.svg");
 }
 
 #panelMenuBookmarkThisPage[starred] {
   list-style-image: url("chrome://browser/skin/bookmark.svg");
 }
 
 #bookmarks-menu-button[cui-areatype="menu-panel"],
-toolbarpaletteitem[place="palette"] > #bookmarks-menu-button {
+toolbarpaletteitem[place="palette"] > #bookmarks-menu-button,
+#panelMenu_toggleBookmarksMenu {
   list-style-image: url("chrome://browser/skin/bookmark-star-on-tray.svg");
 }
 
 #appMenuClearRecentHistory {
   list-style-image: url("chrome://browser/skin/forget.svg");
 }
 
 #appMenuRestoreLastSession {