Bug 947344: add more shortcut tooltip and label helpers where possible. r=mconley.
authorMike de Boer <mdeboer@mozilla.com>
Thu, 08 May 2014 11:14:20 +0200
changeset 190925 992fc815a8079bde3792aba13ab3a588549e129b
parent 190895 c2ba65436960fe9f63ea3d0102dbfd730ab0f50c
child 190946 a4351383899c297e3dc8d55520fa1d85b41c7dd0
push idunknown
push userunknown
push dateunknown
reviewersmconley
bugs947344
milestone32.0a1
Bug 947344: add more shortcut tooltip and label helpers where possible. r=mconley.
browser/base/content/browser-places.js
browser/base/content/browser.xul
browser/components/customizableui/content/panelUI.inc.xul
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/src/CustomizableWidgets.jsm
toolkit/modules/ShortcutUtils.jsm
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1539,16 +1539,20 @@ let BookmarkingUI = {
     this._updateBookmarkPageMenuItem();
     // Update checked status of the toolbar toggle.
     let viewToolbar = document.getElementById("panelMenu_viewBookmarksToolbar");
     let personalToolbar = document.getElementById("PersonalToolbar");
     if (personalToolbar.collapsed)
       viewToolbar.removeAttribute("checked");
     else
       viewToolbar.setAttribute("checked", "true");
+    // Get all statically placed buttons to supply them with keyboard shortcuts.
+    let staticButtons = viewToolbar.parentNode.getElementsByTagName("toolbarbutton");
+    for (let i = 0, l = staticButtons.length; i < l; ++i)
+      CustomizableUI.addShortcut(staticButtons[i]);
     // Setup the Places view.
     this._panelMenuView = new PlacesPanelMenuView("place:folder=BOOKMARKS_MENU",
                                                   "panelMenu_bookmarksMenu",
                                                   "panelMenu_bookmarksMenu", {
                                                     extraClasses: {
                                                       entry: "subviewbutton",
                                                       footer: "panel-subview-footer"
                                                     }
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -862,32 +862,34 @@
                       key="manBookmarkKb"/>
           </menupopup>
         </toolbarbutton>
 
         <!-- This is a placeholder for the Downloads Indicator.  It is visible
              during the customization of the toolbar, in the palette, and before
              the Downloads Indicator overlay is loaded. -->
         <toolbarbutton id="downloads-button" class="toolbarbutton-1 chromeclass-toolbar-additional"
+                       key="key_openDownloads"
                        oncommand="DownloadsIndicatorView.onCommand(event);"
                        ondrop="DownloadsIndicatorView.onDrop(event);"
                        ondragover="DownloadsIndicatorView.onDragOver(event);"
                        ondragenter="DownloadsIndicatorView.onDragOver(event);"
                        label="&downloads.label;"
                        removable="true"
                        cui-areatype="toolbar"
                        tooltip="dynamic-shortcut-tooltip"/>
 
         <toolbarbutton id="home-button" class="toolbarbutton-1 chromeclass-toolbar-additional"
                        persist="class" removable="true"
                        label="&homeButton.label;"
                        ondragover="homeButtonObserver.onDragOver(event)"
                        ondragenter="homeButtonObserver.onDragOver(event)"
                        ondrop="homeButtonObserver.onDrop(event)"
                        ondragexit="homeButtonObserver.onDragExit(event)"
+                       key="goHome"
                        onclick="BrowserGoHome(event);"
                        cui-areatype="toolbar"
                        aboutHomeOverrideTooltip="&abouthome.pageTitle;"/>
 
 
         <toolbarbutton id="social-share-button"
                        class="toolbarbutton-1 chromeclass-toolbar-additional"
                        hidden="true"
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -54,16 +54,17 @@
 
     <panelview id="PanelUI-history" flex="1">
       <label value="&appMenuHistory.label;" class="panel-subview-header"/>
       <vbox class="panel-subview-body">
         <toolbarbutton id="appMenuViewHistorySidebar"
                        label="&appMenuHistory.viewSidebar.label;"
                        type="checkbox"
                        class="subviewbutton"
+                       key="key_gotoHistory"
                        oncommand="toggleSidebar('viewHistorySidebar'); PanelUI.hide();">
           <observes element="viewHistorySidebar" attribute="checked"/>
         </toolbarbutton>
         <toolbarbutton id="appMenuClearRecentHistory"
                        label="&appMenuHistory.clearRecent.label;"
                        class="subviewbutton"
                        command="Tools:Sanitize"/>
 #ifdef MOZ_SERVICES_SYNC
@@ -97,16 +98,17 @@
                        class="subviewbutton"
                        observes="bookmarkThisPageBroadcaster"
                        command="Browser:AddBookmarkAs"
                        onclick="PanelUI.hide();"/>
         <toolbarseparator/>
         <toolbarbutton id="panelMenu_viewBookmarksSidebar"
                        label="&viewBookmarksSidebar2.label;"
                        class="subviewbutton"
+                       key="viewBookmarksSidebarKb"
                        oncommand="toggleSidebar('viewBookmarksSidebar'); PanelUI.hide();">
           <observes element="viewBookmarksSidebar" attribute="checked"/>
         </toolbarbutton>
         <toolbarbutton id="panelMenu_viewBookmarksToolbar"
                        label="&viewBookmarksToolbar.label;"
                        type="checkbox"
                        toolbarId="PersonalToolbar"
                        class="subviewbutton"
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -1239,16 +1239,42 @@ let CustomizableUIInternal = {
     } catch(ex) {
       if (!def) {
         ERROR("Could not localize property '" + name + "'.");
       }
     }
     return def;
   },
 
+  addShortcut: function(aShortcutNode, aTargetNode) {
+    if (!aTargetNode)
+      aTargetNode = aShortcutNode;
+    let document = aShortcutNode.ownerDocument;
+
+    // Detect if we've already been here before.
+    if (!aTargetNode || aTargetNode.hasAttribute("shortcut"))
+      return;
+
+    let shortcutId = aShortcutNode.getAttribute("key");
+    let shortcut;
+    if (shortcutId) {
+      shortcut = document.getElementById(shortcutId);
+    } else {
+      let commandId = aShortcutNode.getAttribute("command");
+      if (commandId)
+        shortcut = ShortcutUtils.findShortcut(document.getElementById(commandId));
+    }
+    if (!shortcut) {
+      ERROR("Could not find a keyboard shortcut for '" + aShortcutNode.outerHTML + "'.");
+      return;
+    }
+
+    aTargetNode.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(shortcut));
+  },
+
   handleWidgetCommand: function(aWidget, aNode, aEvent) {
     LOG("handleWidgetCommand");
 
     if (aWidget.type == "button") {
       if (aWidget.onCommand) {
         try {
           aWidget.onCommand.call(null, aEvent);
         } catch (e) {
@@ -3246,16 +3272,28 @@ this.CustomizableUI = {
    *           otherwise we'll return the empty string
    *
    */
   getLocalizedProperty: function(aWidget, aProp, aFormatArgs, aDef) {
     return CustomizableUIInternal.getLocalizedProperty(aWidget, aProp,
       aFormatArgs, aDef);
   },
   /**
+   * Utility function to detect, find and set a keyboard shortcut for a menuitem
+   * or (toolbar)button.
+   *
+   * @param aShortcutNode the XUL node where the shortcut will be derived from;
+   * @param aTargetNode   (optional) the XUL node on which the `shortcut`
+   *                      attribute will be set. If NULL, the shortcut will be
+   *                      set on aShortcutNode;
+   */
+  addShortcut: function(aShortcutNode, aTargetNode) {
+    return CustomizableUIInternal.addShortcut(aShortcutNode, aTargetNode);
+  },
+  /**
    * Given a node, walk up to the first panel in its ancestor chain, and
    * close it.
    *
    * @param aNode a node whose panel should be closed;
    */
   hidePanelForNode: function(aNode) {
     CustomizableUIInternal.hidePanelForNode(aNode);
   },
--- a/browser/components/customizableui/src/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/src/CustomizableWidgets.jsm
@@ -76,28 +76,16 @@ function updateCombinedWidgetStyle(aNode
   attrs["cui-areatype"] = aArea ? CustomizableUI.getAreaType(aArea) : null;
   for (let i = 0, l = aNode.childNodes.length; i < l; ++i) {
     if (aNode.childNodes[i].localName == "separator")
       continue;
     setAttributes(aNode.childNodes[i], attrs);
   }
 }
 
-function addShortcut(aNode, aDocument, aItem) {
-  let shortcutId = aNode.getAttribute("key");
-  if (!shortcutId) {
-    return;
-  }
-  let shortcut = aDocument.getElementById(shortcutId);
-  if (!shortcut) {
-    return;
-  }
-  aItem.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(shortcut));
-}
-
 function fillSubviewFromMenuItems(aMenuItems, aSubview) {
   let attrs = ["oncommand", "onclick", "label", "key", "disabled",
                "command", "observes", "hidden", "class", "origin",
                "image", "checked"];
 
   let doc = aSubview.ownerDocument;
   let fragment = doc.createDocumentFragment();
   for (let menuChild of aMenuItems) {
@@ -109,17 +97,17 @@ function fillSubviewFromMenuItems(aMenuI
       // Don't insert duplicate or leading separators. This can happen if there are
       // menus (which we don't copy) above the separator.
       if (!fragment.lastChild || fragment.lastChild.localName == "menuseparator") {
         continue;
       }
       subviewItem = doc.createElementNS(kNSXUL, "menuseparator");
     } else if (menuChild.localName == "menuitem") {
       subviewItem = doc.createElementNS(kNSXUL, "toolbarbutton");
-      addShortcut(menuChild, doc, subviewItem);
+      CustomizableUI.addShortcut(menuChild, subviewItem);
     } else {
       continue;
     }
     for (let attr of attrs) {
       let attrVal = menuChild.getAttribute(attr);
       if (attrVal)
         subviewItem.setAttribute(attr, attrVal);
     }
@@ -167,16 +155,21 @@ const CustomizableWidgets = [{
       let query = PlacesUtils.history.getNewQuery();
 
       let items = doc.getElementById("PanelUI-historyItems");
       // Clear previous history items.
       while (items.firstChild) {
         items.removeChild(items.firstChild);
       }
 
+      // Get all statically placed buttons to supply them with keyboard shortcuts.
+      let staticButtons = items.parentNode.getElementsByTagNameNS(kNSXUL, "toolbarbutton");
+      for (let i = 0, l = staticButtons.length; i < l; ++i)
+        CustomizableUI.addShortcut(staticButtons[i]);
+
       PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase)
                          .asyncExecuteLegacyQueries([query], 1, options, {
         handleResult: function (aResultSet) {
           let onHistoryVisit = function (aUri, aEvent, aItem) {
             doc.defaultView.openUILink(aUri, aEvent);
             CustomizableUI.hidePanelForNode(aItem);
           };
           let fragment = doc.createDocumentFragment();
--- a/toolkit/modules/ShortcutUtils.jsm
+++ b/toolkit/modules/ShortcutUtils.jsm
@@ -95,12 +95,17 @@ let ShortcutUtils = {
         Cu.reportError("Error finding " + keyCode + ": " + ex);
         key = keyCode.replace(/^VK_/, '');
       }
     } else {
       key = aElemKey.getAttribute("key");
       key = key.toUpperCase();
     }
     return elemString + key;
+  },
+
+  findShortcut: function(aElemCommand) {
+    let document = aElemCommand.ownerDocument;
+    return document.querySelector("key[command=\"" + aElemCommand.getAttribute("id") + "\"]");
   }
 };
 
 Object.freeze(ShortcutUtils);