Bug 1204836 - In about:home, make any searchable keypress focus the search box. ui-review=philipp, r=gijs, a=lizzard, l10n=lizzard
authorSteffen Wilberg <steffen.wilberg@web.de>
Sun, 20 Sep 2015 17:59:53 +0200
changeset 296327 c40b583a9860ae1240a75cbf7811d97b7ac62ae5
parent 296326 21ba3bb55cb04f7b0feed36e5e4d90972ffd707b
child 296328 a895735dc41409726214cf0408150ecdf88e5869
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs, lizzard
bugs1204836
milestone43.0a2
Bug 1204836 - In about:home, make any searchable keypress focus the search box. ui-review=philipp, r=gijs, a=lizzard, l10n=lizzard
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");
   },
@@ -477,16 +478,35 @@ var gTests = [
   run: Task.async(function* () {
     let syncButton = gBrowser.selectedBrowser.contentDocument.getElementById("sync");
     yield EventUtils.synthesizeMouseAtCenter(syncButton, {}, gBrowser.contentWindow);
 
     yield promiseTabLoadEvent(gBrowser.selectedTab, null, "load");
     is(gBrowser.currentURI.spec, "about:accounts?entrypoint=abouthome",
       "Entry point should be `abouthome`.");
   })
+},
+{
+  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">