Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r=gijs
authorSteffen Wilberg <steffen.wilberg@web.de>
Sun, 20 Sep 2015 17:59:53 +0200
changeset 265947 aebae265f1ff0f85cc9e2c4f051166f896beb440
parent 265908 3d7532ce81ac571962abc3b99582fe7f5d685192
child 265948 c68d71e149200d32b2d66eab23afe651ef0d2aca
push id29475
push usercbook@mozilla.com
push dateMon, 05 Oct 2015 09:58:08 +0000
treeherdermozilla-central@b56aeea0c470 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs
bugs1204836
milestone44.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 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r=gijs
browser/base/content/abouthome/aboutHome.js
browser/base/content/abouthome/aboutHome.xhtml
browser/base/content/test/general/browser_aboutHome.js
browser/locales/en-US/chrome/browser/aboutHome.dtd
--- a/browser/base/content/abouthome/aboutHome.js
+++ b/browser/base/content/abouthome/aboutHome.js
@@ -16,17 +16,17 @@ const DEFAULT_SNIPPETS_URLS = [
 
 const SNIPPETS_UPDATE_INTERVAL_MS = 14400000; // 4 hours.
 
 // IndexedDB storage constants.
 const DATABASE_NAME = "abouthome";
 const DATABASE_VERSION = 1;
 const DATABASE_STORAGE = "persistent";
 const SNIPPETS_OBJECTSTORE_NAME = "snippets";
-var searchText, findKey;
+var searchText;
 
 // This global tracks if the page has been set up before, to prevent double inits
 var gInitialized = false;
 var gObserver = new MutationObserver(function (mutations) {
   for (let mutation of mutations) {
     if (mutation.attributeName == "snippetsVersion") {
       if (!gInitialized) {
         ensureSnippetsMapThen(loadSnippets);
@@ -50,24 +50,30 @@ window.addEventListener("pageshow", func
   document.dispatchEvent(event);
 });
 
 window.addEventListener("pagehide", function() {
   window.gObserver.disconnect();
   window.removeEventListener("resize", fitToWidth);
 });
 
-// make Accel+f focus the search box
 window.addEventListener("keypress", ev => {
-  // Make Ctrl/Cmd+f focus the search box.
-  let modifiers = ev.ctrlKey + ev.altKey + ev.shiftKey + ev.metaKey;
-  if (ev.getModifierState("Accel") && modifiers == 1 && ev.key == findKey) {
-    searchText.focus();
-    ev.preventDefault();
-  }
+  // focus the search-box on keypress
+  if (document.activeElement.id == "searchText") // unless already focussed
+    return;
+
+  let modifiers = ev.ctrlKey + ev.altKey + ev.metaKey;
+  // ignore Ctrl/Cmd/Alt, but not Shift
+  // also ignore Tab, Insert, PageUp, etc., and Space
+  if (modifiers != 0 || ev.charCode == 0 || ev.charCode == 32)
+    return;
+
+  searchText.focus();
+  // need to send the first keypress outside the search-box manually to it
+  searchText.value += ev.key;
 });
 
 // This object has the same interface as Map and is used to store and retrieve
 // the snippets data.  It is lazily initialized by ensureSnippetsMapThen(), so
 // be sure its callback returned before trying to use it.
 var gSnippetsMap;
 var gSnippetsMapCallbacks = [];
 
@@ -190,17 +196,16 @@ function setupSearch()
   // The "autofocus" attribute doesn't focus the form element
   // immediately when the element is first drawn, so the
   // attribute is also used for styling when the page first loads.
   searchText = document.getElementById("searchText");
   searchText.addEventListener("blur", function searchText_onBlur() {
     searchText.removeEventListener("blur", searchText_onBlur);
     searchText.removeAttribute("autofocus");
   });
-  findKey = searchText.dataset.findkey;
 
   if (!gContentSearchController) {
     gContentSearchController =
       new ContentSearchUIController(searchText, searchText.parentNode,
                                     "abouthome", "homepage");
   }
 }
 
--- a/browser/base/content/abouthome/aboutHome.xhtml
+++ b/browser/base/content/abouthome/aboutHome.xhtml
@@ -38,17 +38,17 @@
     <div class="spacer"/>
     <div id="topSection">
       <div id="brandLogo"></div>
 
       <div id="searchIconAndTextContainer">
         <div id="searchIcon"/>
         <input type="text" name="q" value="" id="searchText" maxlength="256"
                aria-label="&contentSearchInput.label;" autofocus="autofocus"
-               dir="auto" data-findkey="&find.commandkey;"/>
+               dir="auto"/>
         <input id="searchSubmit" type="button" value="" onclick="onSearchSubmit(event)"
                aria-label="&contentSearchSubmit.label;"/>
       </div>
 
       <div id="snippetContainer">
         <div id="defaultSnippets" hidden="true">
           <span id="defaultSnippet1">&abouthome.defaultSnippet1.v1;</span>
           <span id="defaultSnippet2">&abouthome.defaultSnippet2.v1;</span>
--- a/browser/base/content/test/general/browser_aboutHome.js
+++ b/browser/base/content/test/general/browser_aboutHome.js
@@ -415,29 +415,30 @@ var gTests = [
       searchController.selectedIndex = 1;
       EventUtils.synthesizeMouseAtCenter(row, {button: 0}, gBrowser.contentWindow);
       yield loadPromise;
       ok(input.value == "x", "Input value did not change");
     });
   }
 },
 {
-  desc: "Cmd+f should focus the search box in the page",
+  desc: "Pressing any key should focus the search box in the page, and send the key to it",
   setup: function () {},
   run: Task.async(function* () {
     let doc = gBrowser.selectedBrowser.contentDocument;
     let logo = doc.getElementById("brandLogo");
     let searchInput = doc.getElementById("searchText");
 
     EventUtils.synthesizeMouseAtCenter(logo, {});
     isnot(searchInput, doc.activeElement, "Search input should not be the active element.");
 
-    EventUtils.synthesizeKey("f", { accelKey: true });
+    EventUtils.synthesizeKey("a", {});
     yield promiseWaitForCondition(() => doc.activeElement === searchInput);
     is(searchInput, doc.activeElement, "Search input should be the active element.");
+    is(searchInput.value, "a", "Search input should be 'a'.");
   })
 },
 {
   desc: "Cmd+k should focus the search box in the page when the search box in the toolbar is absent",
   setup: function () {
     // Remove the search bar from toolbar
     CustomizableUI.removeWidgetFromArea("search-container");
   },
@@ -486,16 +487,35 @@ var gTests = [
     yield EventUtils.synthesizeMouseAtCenter(syncButton, {}, gBrowser.contentWindow);
 
     let result = yield openPrefsPromise;
     window.openPreferences = oldOpenPrefs;
 
     is(result.pane, "paneSync", "openPreferences should be called with paneSync");
     is(result.params.urlParams.entrypoint, "abouthome", "openPreferences should be called with abouthome entrypoint");
   })
+},
+{
+  desc: "Pressing Space while the Addons button is focussed should activate it",
+  setup: function () {},
+  run: Task.async(function* () {
+    // Skip this test on Mac, because Space doesn't activate the button there.
+    if (navigator.platform.indexOf("Mac") == 0) {
+      return Promise.resolve();
+    }
+
+    info("Waiting for about:addons tab to open...");
+    let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, "about:addons");
+    let addOnsButton = gBrowser.selectedBrowser.contentDocument.getElementById("addons");
+    addOnsButton.focus();
+    EventUtils.synthesizeKey(" ", {});
+    let tab = yield promiseTabOpened;
+    is(tab.linkedBrowser.currentURI.spec, "about:addons", "Should have seen the about:addons tab");
+    yield BrowserTestUtils.removeTab(tab);
+  })
 }
 
 ];
 
 function test()
 {
   waitForExplicitFinish();
   requestLongerTimeout(2);
--- a/browser/locales/en-US/chrome/browser/aboutHome.dtd
+++ b/browser/locales/en-US/chrome/browser/aboutHome.dtd
@@ -32,12 +32,8 @@
 <!ENTITY abouthome.preferencesButtonUnix.label  "Preferences">
 <!ENTITY abouthome.addonsButton.label    "Add-ons">
 <!-- LOCALIZATION NOTE (abouthome.appsButton2.label): This string should be consistent with
      the Apps menu item in the Tools menu (webapps.label in browser.dtd) and the Apps toolbar button in
      Firefox's customization palette (web-apps-button.label in customizableWidgets.properties) -->
 <!ENTITY abouthome.appsButton2.label     "Apps">
 <!ENTITY abouthome.downloadsButton.label "Downloads">
 <!ENTITY abouthome.syncButton.label      "&syncBrand.shortName.label;">
-
-<!-- LOCALIZATION NOTE (find.commandkey): This is the key to use in
-     conjunction with accel (Command on Mac or Ctrl on other platforms) to find -->
-<!ENTITY find.commandkey "f">