Bug 1041678 - CTRL/Command K should goto search bar in new tab if open, rather than opening about:home. r=Gijs, r=adw

browser/base/content/browser.js

@@ -23,6 +23,10 @@ XPCOMUtils.defineLazyModuleGetter(this, "ShortcutUtils",
                                   "resource://gre/modules/ShortcutUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "GMPInstallManager",
                                   "resource://gre/modules/GMPInstallManager.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ContentSearch",
+                                  "resource:///modules/ContentSearch.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AboutHome",
+                                  "resource:///modules/AboutHome.jsm");
 XPCOMUtils.defineLazyServiceGetter(this, "gDNSService",
                                    "@mozilla.org/network/dns-service;1",
                                    "nsIDNSService");
@@ -3095,8 +3099,17 @@ const BrowserSearch = {
     }
 #endif
     let openSearchPageIfFieldIsNotActive = function(aSearchBar) {
-      if (!aSearchBar || document.activeElement != aSearchBar.textbox.inputField)
+      let doc = gBrowser.selectedBrowser.contentDocument;
+      let url = doc.documentURI.toLowerCase();
+      let mm = gBrowser.selectedBrowser.messageManager;
+
+      if (url === "about:home") {
+        AboutHome.focusInput(mm);
+      } else if (url === "about:newtab") {
+        ContentSearch.focusInput(mm);
+      } else if (!aSearchBar || document.activeElement != aSearchBar.textbox.inputField) {
         openUILinkIn("about:home", "current");
+      }
     };
 
     let searchBar = this.searchBar;

browser/base/content/content.js

@@ -106,6 +106,9 @@ let AboutHomeListener = {
       case "AboutHome:Update":
         this.onUpdate(aMessage.data);
         break;
+      case "AboutHome:FocusInput":
+        this.onFocusInput();
+        break;
     }
   },
 
@@ -138,6 +141,7 @@ let AboutHomeListener = {
     doc.documentElement.setAttribute("hasBrowserHandlers", "true");
     let self = this;
     addMessageListener("AboutHome:Update", self);
+    addMessageListener("AboutHome:FocusInput", self);
     addEventListener("click", this.onClick, true);
     addEventListener("pagehide", function onPageHide(event) {
       if (event.target.defaultView.frameElement)
@@ -212,6 +216,10 @@ let AboutHomeListener = {
         break;
     }
   },
+
+  onFocusInput: function () {
+    content.document.getElementById("searchText").focus();
+  },
 };
 AboutHomeListener.init(this);
 

browser/base/content/newtab/search.js

@@ -76,6 +76,10 @@ let gSearch = {
     }
   },
 
+  onFocusInput: function () {
+    this._nodes.text.focus();
+  },
+
   _nodeIDSuffixes: [
     "form",
     "logo",

browser/base/content/test/general/browser_aboutHome.js

@@ -419,6 +419,22 @@ let gTests = [
     });
   }
 },
+{
+  desc: "Cmd+k should focus the search bar element",
+  setup: function () {},
+  run: Task.async(function* () {
+    let doc = gBrowser.selectedTab.linkedBrowser.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("k", { accelKey: true });
+    yield promiseWaitForCondition(() => doc.activeElement === searchInput);
+    is(searchInput, doc.activeElement, "Search input should be the active element.");
+  })
+},
 
 ];
 

browser/base/content/test/general/head.js

@@ -82,6 +82,12 @@ function waitForCondition(condition, nextTest, errorMsg) {
   var moveOn = function() { clearInterval(interval); nextTest(); };
 }
 
+function promiseWaitForCondition(aConditionFn) {
+  let deferred = Promise.defer();
+  waitForCondition(aConditionFn, deferred.resolve, "Condition didn't pass.");
+  return deferred.promise;
+}
+
 function getTestPlugin(aName) {
   var pluginName = aName || "Test Plug-in";
   var ph = Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost);

browser/base/content/test/newtab/browser_newtab_search.js

@@ -186,6 +186,16 @@ function runTests() {
   EventUtils.synthesizeKey("VK_DELETE", {});
   ok(table.hidden, "Search suggestion table hidden");
 
+  // Focus a different element than the search input.
+  let btn = getContentDocument().getElementById("newtab-customize-button");
+  yield promiseClick(btn).then(TestRunner.next);
+
+  isnot(input, getContentDocument().activeElement, "Search input should not be focused");
+  // Test that Ctrl/Cmd + K will focus the input field.
+  EventUtils.synthesizeKey("k", { accelKey: true });
+  yield promiseSearchEvents(["FocusInput"]).then(TestRunner.next);
+  is(input, getContentDocument().activeElement, "Search input should be focused");
+
   // Done.  Revert the current engine and remove the new engines.
   Services.search.currentEngine = oldCurrentEngine;
   yield promiseSearchEvents(["CurrentEngine"]).then(TestRunner.next);

browser/modules/AboutHome.jsm

@@ -251,4 +251,13 @@ let AboutHome = {
       Cu.reportError("Error in AboutHome.sendAboutHomeData: " + x);
     });
   },
+
+  /**
+   * Focuses the search input in the page with the given message manager.
+   * @param  messageManager
+   *         The MessageManager object of the selected browser.
+   */
+  focusInput: function (messageManager) {
+    messageManager.sendAsyncMessage("AboutHome:FocusInput");
+  }
 };

browser/modules/ContentSearch.jsm

@@ -94,6 +94,17 @@ this.ContentSearch = {
     Services.obs.addObserver(this, "browser-search-engine-modified", false);
   },
 
+  /**
+   * Focuses the search input in the page with the given message manager.
+   * @param  messageManager
+   *         The MessageManager object of the selected browser.
+   */
+  focusInput: function (messageManager) {
+    messageManager.sendAsyncMessage(OUTBOUND_MESSAGE, {
+      type: "FocusInput"
+    });
+  },
+
   receiveMessage: function (msg) {
     // Add a temporary event handler that exists only while the message is in
     // the event queue.  If the message's source docshell changes browsers in