bug 441794 — User configurable keyboard shortcuts: review comments from mfinkle
authorDaniel Brooks <db48x@db48x.net>
Fri, 26 Sep 2008 15:55:29 -0500
changeset 64890 e77973fc12208c3854d564b09d2aec690af03c0d
parent 64889 dd834f81b11412d678760851fecae985ed58de41
child 64891 ba7266b719381243a634fd9ec7aa15cc220b3f7b
push id1
push userclegnitto@mozilla.com
push dateTue, 12 Apr 2011 01:19:02 +0000
treeherdermozilla-aurora@0cfe6840e0a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs441794
bug 441794 — User configurable keyboard shortcuts: review comments from mfinkle
mobile/chrome/content/browser-ui.js
mobile/chrome/content/shortcuts.js
mobile/chrome/locale/en-US/shortcuts.properties
--- a/mobile/chrome/content/browser-ui.js
+++ b/mobile/chrome/content/browser-ui.js
@@ -373,16 +373,29 @@ var BrowserUI = {
       else if (newToolbarLeft < 0 && aMode == UIMODE_PANEL)
         newToolbarLeft += tabbarW + sidebarW;
       toolbar.left = newToolbarLeft;
 
       browser.left = newLeft + tabbarW;
       sidebar.left = newLeft + tabbarW + browserW;
       panelUI.left = newLeft + tabbarW + browserW + sidebarW;
       panelUI.width = browserW;
+
+      if (aMode == UIMODE_NONE)
+        Shortcuts.deinit();
+      else
+      {
+        let addons = document.getElementById("addons-container");
+        if (addons.getAttribute("src") == "")
+          addons.setAttribute("src", "chrome://mozapps/content/extensions/extensions.xul");
+        let dloads = document.getElementById("downloads-container");
+        if (dloads.getAttribute("src") == "")
+          dloads.setAttribute("src", "chrome://mozapps/content/downloads/downloads.xul");
+        Shortcuts.init();
+      }
   },
 
   _sizeControls : function(aEvent) {
     var rect = document.getElementById("browser-container").getBoundingClientRect();
     var containerW = rect.right - rect.left;
     var containerH = rect.bottom - rect.top;
     var toolbar = document.getElementById("toolbar-main");
     var toolbarH = toolbar.boxObject.height;
@@ -599,79 +612,66 @@ var BrowserUI = {
     {
       this._showToolbar(true);
       this._editToolbar(false);
 
       bookmark.hidden = true;
       urllist.hidden = true;
 
       this._showPanel(UIMODE_NONE);
-      Shortcuts.dismiss();
     }
     else if (aMode == UIMODE_URLEDIT) {
       this._showToolbar(true);
       this._editToolbar(true);
 
       bookmark.hidden = true;
       urllist.hidden = true;
 
       this._showPanel(UIMODE_NONE);
-      Shortcuts.dismiss();
     }
     else if (aMode == UIMODE_BOOKMARK) {
       this._showToolbar(true);
       this._editToolbar(false);
 
       urllist.hidden = true;
       bookmark.hidden = false;
       bookmark.width = container.boxObject.width;
 
       this._showPanel(UIMODE_NONE);
-      Shortcuts.dismiss();
     }
     else if (aMode == UIMODE_BOOKMARKLIST) {
       this._showToolbar(false);
       this._editToolbar(false);
 
       window.addEventListener("keypress", this.closePopup, false);
 
       bookmark.hidden = true;
       urllist.hidden = false;
       urllist.width = container.boxObject.width;
       urllist.height = container.boxObject.height;
 
       this._showPanel(UIMODE_NONE);
-      Shortcuts.dismiss();
     }
     else if (aMode == UIMODE_PANEL) {
       this._showToolbar(true);
       this._editToolbar(false);
 
       bookmark.hidden = true;
 
-      let addons = document.getElementById("addons-container");
-      if (addons.getAttribute("src") == "")
-        addons.setAttribute("src", "chrome://mozapps/content/extensions/extensions.xul");
-      let dloads = document.getElementById("downloads-container");
-      if (dloads.getAttribute("src") == "")
-        dloads.setAttribute("src", "chrome://mozapps/content/downloads/downloads.xul");
-      Shortcuts.edit();
-
       this._showPanel(aMode);
     }
     else if (aMode == UIMODE_TABS || aMode == UIMODE_CONTROLS) {
       this._showPanel(aMode);
     }
     else if (aMode == UIMODE_NONE) {
       this._showToolbar(false);
       this._edit.reallyClosePopup();
       urllist.hidden = true;
       bookmark.hidden = true;
       this._showPanel(aMode);
-      Shortcuts.dismiss();
     }
   },
 
   _showPlaces : function(aItems) {
     var list = document.getElementById("urllist-items");
     while (list.firstChild) {
       list.removeChild(list.firstChild);
     }
@@ -795,17 +795,16 @@ var BrowserUI = {
       case "cmd_back":
       case "cmd_forward":
       case "cmd_reload":
       case "cmd_stop":
       case "cmd_search":
       case "cmd_go":
       case "cmd_star":
       case "cmd_bookmarks":
-      case "cmd_shortcuts":
       case "cmd_menu":
       case "cmd_newTab":
       case "cmd_closeTab":
       case "cmd_actions":
       case "cmd_panel":
       case "cmd_sanitize":
         isSupported = true;
         break;
@@ -864,20 +863,16 @@ var BrowserUI = {
           this.show(UIMODE_BOOKMARK);
           BookmarkHelper.edit(bookmarkURI);
         }
         break;
       }
       case "cmd_bookmarks":
         this.showBookmarks();
         break;
-      case "cmd_shortcuts":
-        this.show(PANELMODE_NONE);
-        Shortcuts.edit();
-        break;
       case "cmd_menu":
         break;
       case "cmd_newTab":
         this.newTab();
         break;
       case "cmd_closeTab":
         Browser.content.removeTab(Browser.content.browser);
         break;
--- a/mobile/chrome/content/shortcuts.js
+++ b/mobile/chrome/content/shortcuts.js
@@ -186,17 +186,17 @@ function ShortcutEditor()
 
     var modifierFlags = { alt: 1, control: 2, meta: 4, shift: 8 };
     function getFlagsForModifiers(modifiers)
     {
         if (!modifiers)
             return 0;
 
         var result;
-        for each (m in modifiers.split(" "))
+        for each (m in modifiers.split(/,\s*|\s+/))
             result |= modifierFlags[m];
         return result;
     }
 
     function getModifiersFromFlags(flags)
     {
         var result = [], i = 1;
         for each (m in ["alt", "control", "meta", "shift"])
@@ -403,19 +403,21 @@ function ShortcutEditor()
             cell.setAttribute("value", keySpec);
             var command = cell.previousSibling.getAttribute("value");
             keySpec = keySpec ? nsIJSON.decode(keySpec) : makeKeySpec();
             addKey(command, keySpec);
             save(command, keySpec);
         }
     }
 
-    // show the window
-    this.edit = function()
+    this.init = function()
     {
+        if (tree)
+            return; // we've already been called
+
         tree = document.getElementById("shortcuts");
 
         var textbox = document.getAnonymousElementByAttribute(tree, "anonid", "input");
         textbox.addEventListener("keypress", keyListener, true);
         textbox.addEventListener("reset", resetListener, true);
         tree.addEventListener("DOMAttrModified", modificationListener, true);
 
         fillShortcutList();
@@ -426,26 +428,26 @@ function ShortcutEditor()
         // TODO: this is a hack, so I want to remove it. to do so, key elements
         // will have to respond to direct dom manipulation.
         Array.map(document.getElementsByTagNameNS(XUL_NS, "keyset"),
                   function(e) { return e.parentNode.removeChild(e); })
              .forEach(function(e) { document.documentElement.appendChild(e); });
         keyCache = undefined;
     }
 
-    this.dismiss = function()
+    this.deinit = function()
     {
         if (!tree)
-            return;
+            return; // got called early, just return
 
         hack();
         var textbox = document.getAnonymousElementByAttribute(tree, "anonid", "input");
         textbox.removeEventListener("keypress", keyListener, true);
         textbox.removeEventListener("reset", resetListener, true);
-        document.getElementById("shortcuts-container").hidden = true;
+        tree.removeEventListener("DOMAttrModified", modificationListener, true);
     };
 
     // also, updating the UI is helpful
     function fillShortcutList()
     {
         var commands = getCommandNames();
         var sb = document.getElementById("shortcut-bundles").childNodes;
 
@@ -542,20 +544,20 @@ function ShortcutEditor()
                 dump("ERROR FAILURE: "+ msg +"\n");
         }
 
         [[[undefined, "a"],                               {exists: true,  modifiers: 0,  key: "a",   keycode: false},      "A"],
          [["alt", "a"],                                   {exists: true,  modifiers: 1,  key: "a",   keycode: false},      "Alt+A"],
          [["control", "a"],                               {exists: true,  modifiers: 2,  key: "a",   keycode: false},      "Ctrl+A"],
          [["meta", "a"],                                  {exists: true,  modifiers: 4,  key: "a",   keycode: false},      "Meta+A"],
          [["shift", "a"],                                 {exists: true,  modifiers: 8,  key: "a",   keycode: false},      "Shift+A"],
-         [["control alt", "a"],                           {exists: true,  modifiers: 3,  key: "a",   keycode: false},      "Ctrl+Alt+A"],
-         [["alt shift", "a"],                             {exists: true,  modifiers: 9,  key: "a",   keycode: false},      "Alt+Shift+A"],
-         [["shift meta", "a"],                            {exists: true,  modifiers: 12, key: "a",   keycode: false},      "Meta+Shift+A"],
-         [["control alt shift", "a"],                     {exists: true,  modifiers: 11, key: "a",   keycode: false},      "Ctrl+Alt+Shift+A"],
+         [["control,alt", "a"],                           {exists: true,  modifiers: 3,  key: "a",   keycode: false},      "Ctrl+Alt+A"],
+         [["alt, shift", "a"],                            {exists: true,  modifiers: 9,  key: "a",   keycode: false},      "Alt+Shift+A"],
+         [["shift ,meta", "a"],                           {exists: true,  modifiers: 12, key: "a",   keycode: false},      "Meta+Shift+A"],
+         [["control , alt shift", "a"],                   {exists: true,  modifiers: 11, key: "a",   keycode: false},      "Ctrl+Alt+Shift+A"],
          [["alt shift meta", "a"],                        {exists: true,  modifiers: 13, key: "a",   keycode: false},      "Alt+Meta+Shift+A"],
          [[undefined, undefined, "VK_BACK"],              {exists: true,  modifiers: 0,  key: false, keycode: "VK_BACK"},  "Backspace"],
          [["control", undefined, "VK_BACK"],              {exists: true,  modifiers: 2,  key: false, keycode: "VK_BACK"},  "Ctrl+Backspace"],
          [["control", undefined, "VK_A"],                 {exists: true,  modifiers: 2,  key: false, keycode: "VK_A"},     "Ctrl+A"],
          [["meta shift alt control", undefined, "VK_A"],  {exists: true,  modifiers: 15, key: false, keycode: "VK_A"},     "Ctrl+Alt+Meta+Shift+A"],
          [[],                                             {exists: false, modifiers: 0,  key: false, keycode: false},      ""],
          [["control"],                                    {exists: false, modifiers: 2,  key: false, keycode: false},      ""],
          [["foobar", "a"],                                {exists: true,  modifiers: 0,  key: "a",   keycode: false},      "A"],
--- a/mobile/chrome/locale/en-US/shortcuts.properties
+++ b/mobile/chrome/locale/en-US/shortcuts.properties
@@ -6,26 +6,23 @@ cmd_search.name=Search
 cmd_go.name=Load URL
 cmd_star.name=Star page
 cmd_bookmarks.name=View bookmarks
 cmd_find.name=Find in page
 cmd_findAgain.name=Find again
 cmd_findPrevious.name=Find previous
 cmd_menu.name=Show menu
 cmd_fullscreen.name=Use full screen
-cmd_addons.name=Show addons list
-cmd_downloads.name=Show downloads list
 cmd_scrollPageUp.name=Page up
 cmd_scrollToBeginning.name=Scroll to beginning
 cmd_scrollPageDown.name=Page down
 cmd_scrollToEnd.name=Scroll to end
 cmd_cut.name=Cut
 cmd_copy.name=Copy
 cmd_copylink.name=Copy link URL
 cmd_paste.name=Paste
 cmd_delete.name=Delete
 cmd_selectAll.name=Select All
-cmd_shortcuts.name=Edit shortcuts
 cmd_newTab.name=Open new tab
 cmd_closeTab.name=Close current tab
 cmd_actions.name=Open Actions list
 cmd_panel.name=Open panel of stuff
 cmd_sanitize.name=Delete all saved personal information