Bug 564328 - Keyboard shortcut Ctrl+F conflict and cmd_find ambiguity, used for both Find in This Message and Quick Filter; r=asuth

mail/base/content/quickFilterBar.js

@@ -467,7 +467,7 @@ let QuickFilterBarMuxer = {
       return null;
 
     if (aCommand == "cmd_popQuickFilterBarStack" ||
-        aCommand == "cmd_find" ||
+        aCommand == "cmd_showQuickFilterBar" ||
         aCommand == "cmd_toggleQuickFilterBar")
       return true;
     else
@@ -479,7 +479,7 @@ let QuickFilterBarMuxer = {
       return null;
 
     if (aCommand == "cmd_popQuickFilterBarStack" ||
-        aCommand == "cmd_find" ||
+        aCommand == "cmd_showQuickFilterBar" ||
         aCommand == "cmd_toggleQuickFilterBar")
       return true;
     else
@@ -494,15 +494,14 @@ let QuickFilterBarMuxer = {
       QuickFilterBarMuxer.cmdEscapeFilterStack();
       return true;
     }
-    else if (aCommand == "cmd_find") {
+    else if (aCommand == "cmd_showQuickFilterBar") {
       let textWidget = document.getElementById(
                          QuickFilterManager.textBoxDomId);
-      // if it's not already focused, then focus/select it
-      if (document.commandDispatcher.focusedElement != textWidget.inputField) {
+
+      if (this.activeFilterer.visible == false)
         QuickFilterBarMuxer._showFilterBar(true);
-        textWidget.select();
-        return true;
-      }
+      textWidget.select();
+      return true;
     }
     else if (aCommand == "cmd_toggleQuickFilterBar") {
       this._showFilterBar(!this.activeFilterer.visible);
@@ -596,6 +595,8 @@ let QuickFilterBarMuxer = {
     if (!aShow) {
       this.activeFilterer.clear();
       this.updateSearch();
+      let threadPane = document.getElementById("threadTree");
+      threadPane.focus();
     }
     this.reflectFiltererState(this.activeFilterer,
                               this.tabmail.currentTabInfo.folderDisplay);

mail/base/content/quickFilterBar.xul

@@ -44,9 +44,15 @@
   <keyset id="mailKeys">
     <key id="key_qfb_escape" keycode="VK_ESCAPE"
          command="cmd_popQuickFilterBarStack"/>
+    <key id="key_qfb_show" key="&quickFilterBar.show.key2;"
+         modifiers="accel,shift"
+         command="cmd_showQuickFilterBar"/>
   </keyset>
 
   <commandset id="mailViewMenuItems">
+    <command id="cmd_showQuickFilterBar"
+             oncommand="goDoCommand('cmd_showQuickFilterBar');"
+             />
     <command id="cmd_toggleQuickFilterBar"
              oncommand="goDoCommand('cmd_toggleQuickFilterBar');"
              />
@@ -63,7 +69,6 @@
               accesskey="&quickFilterBar.toggleBarVisibility.menu.accesskey;"
               command="cmd_toggleQuickFilterBar"
               observes="qfb-show-filter-bar"
-              key="key_find"
               />
   </menupopup>
 

mail/locales/en-US/chrome/messenger/quickFilterBar.dtd

@@ -150,23 +150,25 @@
 <!ENTITY quickFilterBar.textbox.emptyText.base
          "Filter these messages... #1">
 
+<!-- LOCALIZATION NOTE (quickFilterBar.show.key2):
+     This is the key used to show the quick filter bar. -->
+<!ENTITY quickFilterBar.show.key2
+         "k">
+
 <!-- LOCALIZATION NOTE (quickFilterBar.textbox.emptyText.keyLabel.nonmac):
      The description of the key-binding to get into the box on windows and
-     linux (which use the control key).  We use the keybinding for cmd_find
-     used by the find-in-message mechanism, so that's the letter to indicate
-     if you don't use 'f' for your localization.
+     linux (which use the control key).  This should match the value of
+     quickFilterBar.show.key above.
      -->
 <!ENTITY quickFilterBar.textbox.emptyText.keyLabel.nonmac
-         "&lt;Ctrl+F&gt;">
+         "&lt;Ctrl+Shift+K&gt;">
 
 <!-- LOCALIZATION NOTE (quickFilterBar.textbox.emptyText.keyLabel.mac):
      The description of the key-binding to get into the box on mac systems.
-     We use the keybinding for cmd_find used by the find-in-message mechanism,
-     so that's the letter to indicate  if you don't use 'f' for your
-     localization.
+     This should match the value of quickFilterBar.show.key above.
      -->
 <!ENTITY quickFilterBar.textbox.emptyText.keyLabel1.mac
-         "&lt;&#x2318;F&gt;">
+         "&lt;&#x2318;Shift+K&gt;">
 
 <!-- LOCALIZATION NOTE (quickFilterBar.textbox.idealWidth):
      The number of pixels for the ideal width of the quick filter box textbox.

mail/test/mozmill/quick-filter-bar/test-keyboard-interface.js

@@ -36,9 +36,9 @@
 
 /*
  * Tests keyboard stuff that doesn't fall under some other test's heading.
- * Namely, control-f toggling the bar into existence happens in
- * test-toggle-bar.js, but we test that repeatedly hitting control-f toggles
- * between our search text box and the find-in-message text box.
+ * Namely, control-shift-k toggling the bar into existence happens in
+ * test-toggle-bar.js, but we test that repeatedly hitting control-shift-k
+ * selects the text entered in the quick filter bar.
  */
 
 var MODULE_NAME = 'test-keyboard-interface';
@@ -154,33 +154,43 @@ function test_escape_does_not_reach_us_from_gloda_search() {
 }
 
 /**
- * Control-f jumps to the quickfilter text box when not in it.  when in it, it
- * allows the normal find-in-message command to execute, which should jump it
- * to the find-in-message textbox thing.
+ * Control-shift-k expands the quick filter bar when it's collapsed. When
+ * already expanded, it focuses the text box and selects its text.
  */
-function test_control_f_toggles_between_textboxes() {
+function test_control_shift_k_shows_quick_filter_bar() {
   let dispatcha = mc.window.document.commandDispatcher;
+  let qfbTextbox = mc.e("qfb-qs-textbox");
 
   // focus explicitly on the thread pane so we know where the focus is.
   mc.e("threadTree").focus();
   // select a message so we can find in message
   select_click_row(0);
 
-  // hit control-f to get in the quick filter box
-  mc.keypress(null, "f", {accelKey: true});
-  if (dispatcha.focusedElement != mc.e("qfb-qs-textbox").inputField)
-    throw new Error("control-f did not focus quick filter textbox");
+  // hit control-shift-k to get in the quick filter box
+  mc.keypress(null, "k", {accelKey: true, shiftKey: true});
+  if (dispatcha.focusedElement != qfbTextbox.inputField)
+    throw new Error("control-shift-k did not focus quick filter textbox");
 
-  // hit control-f to get in the find-in-message box
-  mc.keypress(null, "f", {accelKey: true});
-  if (dispatcha.focusedElement != mc.e("FindToolbar")._findField.inputField)
-    throw new Error("control-f did not focus toolbar textbox. focused elem: " +
-                    dispatcha.focusedElement + " expected elem: " +
-                    mc.e("FindToolbar")._findField.inputField + " last dude: " +
-                    mc.e("qfb-qs-textbox").inputField);
+  set_filter_text("search string");
 
-  // secret bonus test!  hit escape and make sure it only closes the
-  //  find-in-message bar.
+  // hit control-shift-k to select the text in the quick filter box
+  mc.keypress(null, "k", {accelKey: true, shiftKey: true});
+  if (dispatcha.focusedElement != qfbTextbox.inputField)
+    throw new Error("second control-shift-k did not keep focus on filter " +
+                    "textbox");
+  if (qfbTextbox.selectionStart != 0 ||
+      qfbTextbox.selectionEnd != qfbTextbox.textLength)
+    throw new Error("second control-shift-k did not select text in filter " +
+                    "textbox");
+
+  // hit escape and make sure the text is cleared, but the quick filter bar is
+  // still open.
   mc.keypress(null, "VK_ESCAPE", {});
   assert_quick_filter_bar_visible(true);
+  assert_filter_text("");
+
+  // hit escape one more time and make sure we finally collapsed the quick
+  // filter bar.
+  mc.keypress(null, "VK_ESCAPE", {});
+  assert_quick_filter_bar_visible(false);
 }

mail/test/mozmill/quick-filter-bar/test-toggle-bar.js

@@ -82,7 +82,7 @@ function test_direct_toggle() {
   assert_quick_filter_bar_visible(true);
 }
 
-function test_control_f_triggers_display() {
+function test_control_shift_k_triggers_display() {
   // hide it
   toggle_quick_filter_bar();
   assert_quick_filter_bar_visible(false);
@@ -90,8 +90,8 @@ function test_control_f_triggers_display() {
   // focus explicitly on the thread pane so we know where the focus is.
   mc.e("threadTree").focus();
 
-  // hit control-f
-  mc.keypress(null, "f", {accelKey: true});
+  // hit control-shift-k
+  mc.keypress(null, "k", {accelKey: true, shiftKey: true});
 
   // now we should be visible again!
   assert_quick_filter_bar_visible(true);