Bug 1659131 - Top Site search shortcuts should enter search mode. r=adw

Differential Revision: https://phabricator.services.mozilla.com/D87908

browser/components/newtab/lib/PlacesFeed.jsm

@@ -367,7 +367,7 @@ class PlacesFeed {
   }
 
   fillSearchTopSiteTerm({ _target, data }) {
-    _target.browser.ownerGlobal.gURLBar.search(`${data.label} `);
+    _target.browser.ownerGlobal.gURLBar.searchWithAlias(data.label);
   }
 
   _getSearchPrefix(isPrivateWindow) {
@@ -391,8 +391,7 @@ class PlacesFeed {
     if (!data || !data.text) {
       urlBar.setHiddenFocus();
     } else {
-      // Pass the provided text to the awesomebar. Prepend the @engine shortcut.
-      urlBar.search(`${searchAlias}${data.text}`);
+      urlBar.searchWithAlias(searchAlias, data.text);
       isFirstChange = false;
     }
 
@@ -403,7 +402,7 @@ class PlacesFeed {
       if (isFirstChange) {
         isFirstChange = false;
         urlBar.removeHiddenFocus();
-        urlBar.search(searchAlias);
+        urlBar.searchWithAlias(searchAlias);
         this.store.dispatch(
           ac.OnlyToOneContent({ type: at.HIDE_SEARCH }, meta.fromTarget)
         );

browser/components/newtab/test/browser/browser_topsites_section.js

@@ -193,9 +193,26 @@ test_newtab({
       gURLBar.focused,
       "We clicked a search topsite the focus should be in location bar"
     );
-    ok(
-      gURLBar.value.includes(searchTopSiteTag),
-      "Should contain the tag of the search topsite clicked"
-    );
+    if (!Services.prefs.getBoolPref("browser.urlbar.update2")) {
+      ok(
+        gURLBar.value.includes(searchTopSiteTag),
+        "Should contain the tag of the search topsite clicked"
+      );
+    } else {
+      let engineName = Services.search.getEngineByAlias(searchTopSiteTag).name;
+      // We don't use UrlbarTestUtils.assertSearchMode here since the newtab
+      // testing scope doesn't integrate well with UrlbarTestUtils.
+      ok(
+        ObjectUtils.deepEqual(
+          gURLBar.searchMode,
+          { engineName },
+          "The Urlbar is in search mode."
+        )
+      );
+      ok(
+        gURLBar.hasAttribute("searchmode"),
+        "The Urlbar has the searchmode attribute."
+      );
+    }
   },
 });

browser/components/newtab/test/browser/head.js

@@ -1,5 +1,10 @@
 "use strict";
 
+ChromeUtils.defineModuleGetter(
+  this,
+  "ObjectUtils",
+  "resource://gre/modules/ObjectUtils.jsm"
+);
 ChromeUtils.defineModuleGetter(
   this,
   "PlacesTestUtils",

browser/components/newtab/test/unit/lib/PlacesFeed.test.js

@@ -323,7 +323,7 @@ describe("PlacesFeed", () => {
       assert.calledOnce(feed.fillSearchTopSiteTerm);
     });
     it("should set the URL bar value to the label value", () => {
-      const locationBar = { search: sandbox.stub() };
+      const locationBar = { searchWithAlias: sandbox.stub() };
       const action = {
         type: at.FILL_SEARCH_TERM,
         data: { label: "@Foo" },
@@ -332,8 +332,8 @@ describe("PlacesFeed", () => {
 
       feed.fillSearchTopSiteTerm(action);
 
-      assert.calledOnce(locationBar.search);
-      assert.calledWithExactly(locationBar.search, "@Foo ");
+      assert.calledOnce(locationBar.searchWithAlias);
+      assert.calledWithExactly(locationBar.searchWithAlias, "@Foo");
     });
     it("should call saveToPocket on SAVE_TO_POCKET", () => {
       const action = {
@@ -497,7 +497,7 @@ describe("PlacesFeed", () => {
     beforeEach(() => {
       fakeUrlBar = {
         focus: sinon.spy(),
-        search: sinon.spy(),
+        searchWithAlias: sinon.spy(),
         setHiddenFocus: sinon.spy(),
         removeHiddenFocus: sinon.spy(),
         addEventListener: (ev, cb) => {
@@ -514,12 +514,12 @@ describe("PlacesFeed", () => {
         meta: { fromTarget: {} },
       });
       assert.calledOnce(fakeUrlBar.setHiddenFocus);
-      assert.notCalled(fakeUrlBar.search);
+      assert.notCalled(fakeUrlBar.searchWithAlias);
       assert.notCalled(feed.store.dispatch);
 
       // Now type a character.
       listeners.keydown({ key: "f" });
-      assert.calledOnce(fakeUrlBar.search);
+      assert.calledOnce(fakeUrlBar.searchWithAlias);
       assert.calledOnce(fakeUrlBar.removeHiddenFocus);
       assert.calledOnce(feed.store.dispatch);
       assert.calledWith(feed.store.dispatch, {
@@ -538,8 +538,8 @@ describe("PlacesFeed", () => {
         data: { text: "foo" },
         meta: { fromTarget: {} },
       });
-      assert.calledOnce(fakeUrlBar.search);
-      assert.calledWith(fakeUrlBar.search, "@google foo");
+      assert.calledOnce(fakeUrlBar.searchWithAlias);
+      assert.calledWith(fakeUrlBar.searchWithAlias, "@google ", "foo");
       assert.notCalled(fakeUrlBar.focus);
       assert.notCalled(fakeUrlBar.setHiddenFocus);
 
@@ -563,8 +563,8 @@ describe("PlacesFeed", () => {
         data: { text: "foo" },
         meta: { fromTarget: {} },
       });
-      assert.calledOnce(fakeUrlBar.search);
-      assert.calledWith(fakeUrlBar.search, "@bing foo");
+      assert.calledOnce(fakeUrlBar.searchWithAlias);
+      assert.calledWith(fakeUrlBar.searchWithAlias, "@bing ", "foo");
       assert.notCalled(fakeUrlBar.focus);
       assert.notCalled(fakeUrlBar.setHiddenFocus);
 
@@ -588,8 +588,8 @@ describe("PlacesFeed", () => {
         data: { text: "foo" },
         meta: { fromTarget: {} },
       });
-      assert.calledOnce(fakeUrlBar.search);
-      assert.calledWith(fakeUrlBar.search, "@google foo");
+      assert.calledOnce(fakeUrlBar.searchWithAlias);
+      assert.calledWithExactly(fakeUrlBar.searchWithAlias, "@google ", "foo");
       assert.notCalled(fakeUrlBar.focus);
 
       // Now call ESC keydown.
@@ -612,8 +612,8 @@ describe("PlacesFeed", () => {
         data: { text: "foo" },
         meta: { fromTarget: {} },
       });
-      assert.calledOnce(fakeUrlBar.search);
-      assert.calledWith(fakeUrlBar.search, "foo");
+      assert.calledOnce(fakeUrlBar.searchWithAlias);
+      assert.calledWithExactly(fakeUrlBar.searchWithAlias, "", "foo");
     });
   });
 

browser/components/urlbar/UrlbarInput.jsm

@@ -1162,6 +1162,32 @@ class UrlbarInput {
     this.inputField.dispatchEvent(event);
   }
 
+  /**
+   * Starts a search with a given alias. If update2 is enabled, search mode is
+   * entered. Otherwise, this just calls search().
+   * @param {string} alias
+   *   A search engine alias.
+   * @param {string} [value]
+   *   The input's value will be set to this value, and the search will
+   *   use it as its query.
+   */
+  searchWithAlias(alias, value = "") {
+    alias = alias.trim();
+    if (UrlbarPrefs.get("update2")) {
+      // Enter search mode.
+      let engine = Services.search.getEngineByAlias(alias);
+      if (engine) {
+        this.setSearchMode({ engineName: engine.name });
+        this.search(value);
+      } else {
+        this.search(`${alias} ${value}`);
+      }
+    } else {
+      // Pass the provided text to the Urlbar. Prepend the @engine shortcut.
+      this.search(`${alias} ${value}`);
+    }
+  }
+
   /**
    * Focus without the focus styles.
    * This is used by Activity Stream and about:privatebrowsing for search hand-off.

browser/components/urlbar/tests/browser/browser_searchFunction.js

@@ -5,11 +5,20 @@
 
 "use strict";
 
+const ALIAS = "@enginealias";
+let aliasEngine;
+
 add_task(async function init() {
   // Run this in a new tab, to ensure all the locationchange notifications have
   // fired.
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+
+  aliasEngine = await Services.search.addEngineWithDetails("Test", {
+    alias: ALIAS,
+    template: "http://example.com/?search={searchTerms}",
+  });
   registerCleanupFunction(async function() {
+    await Services.search.removeEngine(aliasEngine);
     BrowserTestUtils.removeTab(tab);
     gURLBar.handleRevert();
   });
@@ -117,6 +126,75 @@ add_task(async function searchIME() {
   await UrlbarTestUtils.promisePopupClose(window);
 });
 
+// Calls searchWithAlias(), a companion function to search().
+add_task(async function searchWithAlias() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.update2", true]],
+  });
+
+  await UrlbarTestUtils.promisePopupOpen(window, () =>
+    gURLBar.searchWithAlias(ALIAS, "test")
+  );
+  Assert.ok(gURLBar.hasAttribute("focused"), "Urlbar is focused");
+  UrlbarTestUtils.assertSearchMode(window, {
+    engineName: aliasEngine.name,
+  });
+  await assertUrlbarValue("test");
+  assertOneOffButtonsVisible(true);
+  await UrlbarTestUtils.exitSearchMode(window, { backspace: true });
+  await UrlbarTestUtils.promisePopupClose(window);
+
+  await SpecialPowers.popPrefEnv();
+});
+
+// Calls searchWithAlias() with update2 disabled.
+add_task(async function searchWithAlias_legacy() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.update2", false]],
+  });
+
+  await UrlbarTestUtils.promisePopupOpen(window, () =>
+    gURLBar.searchWithAlias(ALIAS, "test")
+  );
+  Assert.ok(gURLBar.hasAttribute("focused"), "Urlbar is focused");
+  Assert.equal(gURLBar.value, `${ALIAS} test`);
+  await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0);
+  let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+  Assert.equal(
+    result.type,
+    UrlbarUtils.RESULT_TYPE.SEARCH,
+    "Should have type search for the first result"
+  );
+  Assert.equal(
+    result.searchParams.query,
+    "test",
+    "Should have the correct query for the first result"
+  );
+
+  assertOneOffButtonsVisible(false);
+  await UrlbarTestUtils.promisePopupClose(window);
+
+  await SpecialPowers.popPrefEnv();
+});
+
+// Calls searchWithAlias() with an invalid alias.
+add_task(async function searchWithAliasInvalidAlias() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.urlbar.update2", true]],
+  });
+
+  await UrlbarTestUtils.promisePopupOpen(window, () =>
+    gURLBar.searchWithAlias("@invalidalias", "test")
+  );
+  Assert.ok(gURLBar.hasAttribute("focused"), "Urlbar is focused");
+  UrlbarTestUtils.assertSearchMode(window, null);
+  await assertUrlbarValue("@invalidalias test");
+  assertOneOffButtonsVisible(false);
+  await UrlbarTestUtils.promisePopupClose(window);
+
+  await SpecialPowers.popPrefEnv();
+});
+
 /**
  * Asserts that the one-off search buttons are or aren't visible.
  *