Bug 1578584 - Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
☠☠ backed out by 8b8ce4612836 ☠ ☠
authorDrew Willcoxon <adw@mozilla.com>
Thu, 19 Sep 2019 23:48:12 +0000
changeset 495011 abcfd108c7e5e9819eb796c6dd8748f5a254519c
parent 495010 644e956b4de85cc5d446e5633fb6adce571fc1e5
child 495012 220a3c19ebd3ffdf6bea71fbcb916cf8cc9639ef
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersharry, mixedpuppy
bugs1578584
milestone71.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 1578584 - Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy Adds a new event listener to `browser.urlbar` called `onResultPicked`. This event is fired for tip results when they don't specify a URL. Hypothetically it could be fired for any type of result that didn't specify a URL, but that's only tips for now. The listener is passed two arguments: the payload of the result that was picked, and a "details" object whose properties depend on the type of result. For tips, details is `{ helpPicked }`, where `helpPicked` is true if the help button was picked and false if the main button was picked. Differential Revision: https://phabricator.services.mozilla.com/D46254
browser/components/extensions/parent/ext-urlbar.js
browser/components/extensions/schemas/urlbar.json
browser/components/extensions/test/browser/browser.ini
browser/components/extensions/test/browser/browser_ext_urlbar.js
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarProviderExtension.jsm
browser/components/urlbar/UrlbarProvidersManager.jsm
browser/components/urlbar/UrlbarUtils.jsm
--- a/browser/components/extensions/parent/ext-urlbar.js
+++ b/browser/components/extensions/parent/ext-urlbar.js
@@ -187,16 +187,33 @@ this.urlbar = class extends ExtensionAPI
                   throw context.normalizeError(error);
                 });
               }
             );
             return () => provider.setEventListener("resultsRequested", null);
           },
         }).api(),
 
+        onResultPicked: new EventManager({
+          context,
+          name: "urlbar.onResultPicked",
+          register: (fire, providerName) => {
+            let provider = UrlbarProviderExtension.getOrCreate(providerName);
+            provider.setEventListener(
+              "resultPicked",
+              async (resultPayload, details) => {
+                return fire.async(resultPayload, details).catch(error => {
+                  throw context.normalizeError(error);
+                });
+              }
+            );
+            return () => provider.setEventListener("resultPicked", null);
+          },
+        }).api(),
+
         openViewOnFocus: getSettingsAPI(
           context.extension.id,
           "openViewOnFocus",
           () => UrlbarPrefs.get("openViewOnFocus")
         ),
 
         engagementTelemetry: getSettingsAPI(
           context.extension.id,
--- a/browser/components/extensions/schemas/urlbar.json
+++ b/browser/components/extensions/schemas/urlbar.json
@@ -161,16 +161,41 @@
         ],
         "returns": {
           "type": "array",
           "items": {
             "$ref": "Result"
           },
           "description": "The results that the provider fetched for the query."
         }
+      },
+      {
+        "name": "onResultPicked",
+        "type": "function",
+        "description": "Typically, a provider includes a <code>url</code> property in its results' payloads. When the user picks a result with a URL, Firefox automatically loads the URL. URLs don't make sense for every result type, however. When the user picks a result without a URL, this event is fired. The provider should take an appropriate action in response. Currently the only applicable <code>ResultType</code> is <code>tip</code>.",
+        "parameters": [
+          {
+            "name": "payload",
+            "type": "object",
+            "description": "The payload of the result that was picked."
+          },
+          {
+            "name": "details",
+            "type": "object",
+            "description": "Details about the pick. The specific properties depend on the result type."
+          }
+        ],
+        "extraParameters": [
+          {
+            "name": "providerName",
+            "type": "string",
+            "pattern": "^[a-zA-Z0-9_-]+$",
+            "description": "The listener will be called for the results of the provider with this name."
+          }
+        ]
       }
     ]
   },
   {
     "namespace": "urlbar.contextualTip",
     "description": "A contextual tip appears in the urlbar's view (its search results panel) and has an icon, text, optional button, and an optional link. Use the <code>browser.urlbar.contextualTip</code> API to experiment with the contextual tip. Restricted to Mozilla privileged WebExtensions.",
     "types": [
       {
--- a/browser/components/extensions/test/browser/browser.ini
+++ b/browser/components/extensions/test/browser/browser.ini
@@ -265,16 +265,17 @@ skip-if = os == 'mac' # Save as PDF not 
 [browser_ext_tabs_cookieStoreId.js]
 [browser_ext_tabs_update.js]
 [browser_ext_tabs_update_highlighted.js]
 [browser_ext_tabs_update_url.js]
 [browser_ext_tabs_zoom.js]
 [browser_ext_themes_validation.js]
 [browser_ext_topSites.js]
 [browser_ext_url_overrides_newtab.js]
+[browser_ext_urlbar.js]
 [browser_ext_urlbar_contextual_tip.js]
 [browser_ext_user_events.js]
 [browser_ext_webRequest.js]
 [browser_ext_webrtc.js]
 skip-if = os == 'mac' # Bug 1565738
 [browser_ext_webNavigation_frameId0.js]
 [browser_ext_webNavigation_getFrames.js]
 [browser_ext_webNavigation_onCreatedNavigationTarget.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_urlbar.js
@@ -0,0 +1,215 @@
+"use strict";
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  UrlbarTestUtils: "resource://testing-common/UrlbarTestUtils.jsm",
+});
+
+async function loadExtension(options = {}) {
+  let ext = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["urlbar"],
+    },
+    isPrivileged: true,
+    background() {
+      browser.test.onMessage.addListener(options => {
+        browser.urlbar.onBehaviorRequested.addListener(query => {
+          return "restricting";
+        }, "test");
+        browser.urlbar.onResultsRequested.addListener(query => {
+          return [
+            {
+              type: "tip",
+              source: "local",
+              heuristic: true,
+              payload: {
+                text: "Test",
+                buttonText: "OK",
+                data: "testData",
+                buttonUrl: options.buttonUrl,
+                helpUrl: options.helpUrl,
+              },
+            },
+          ];
+        }, "test");
+        browser.urlbar.onResultPicked.addListener((payload, details) => {
+          browser.test.assertEq(payload.text, "Test", "payload.text");
+          browser.test.assertEq(payload.buttonText, "OK", "payload.buttonText");
+          browser.test.assertEq(payload.data, "testData", "payload.data");
+          browser.test.sendMessage("onResultPicked received", details);
+        }, "test");
+        browser.test.sendMessage("ready");
+      });
+    },
+  });
+  await ext.startup();
+  await Promise.all([ext.sendMessage(options), ext.awaitMessage("ready")]);
+  return ext;
+}
+
+// Loads an extension without a main button URL and presses enter on the main
+// button.
+add_task(async function testOnResultPicked_mainButton_noURL_enter() {
+  let ext = await loadExtension();
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "test",
+  });
+  EventUtils.synthesizeKey("KEY_ArrowDown");
+  EventUtils.synthesizeKey("KEY_Enter");
+  let details = await ext.awaitMessage("onResultPicked received");
+  Assert.deepEqual(details, { helpPicked: false });
+  await ext.unload();
+});
+
+// Loads an extension without a main button URL and clicks the main button.
+add_task(async function testOnResultPicked_mainButton_noURL_mouse() {
+  let ext = await loadExtension();
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "test",
+  });
+  let mainButton = document.querySelector(
+    "#urlbarView-row-0 .urlbarView-tip-button"
+  );
+  Assert.ok(mainButton);
+  EventUtils.synthesizeMouseAtCenter(mainButton, {});
+  let details = await ext.awaitMessage("onResultPicked received");
+  Assert.deepEqual(details, { helpPicked: false });
+  await ext.unload();
+});
+
+// Loads an extension with a main button URL and presses enter on the main
+// button.
+add_task(async function testOnResultPicked_mainButton_url_enter() {
+  let ext = await loadExtension({ buttonUrl: "http://example.com/" });
+  await BrowserTestUtils.withNewTab("about:blank", async () => {
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      waitForFocus,
+      value: "test",
+    });
+    let loadedPromise = BrowserTestUtils.browserLoaded(
+      gBrowser.selectedBrowser
+    );
+    ext.onMessage("onResultPicked received", () => {
+      Assert.ok(false, "onResultPicked should not be called");
+    });
+    EventUtils.synthesizeKey("KEY_ArrowDown");
+    EventUtils.synthesizeKey("KEY_Enter");
+    await loadedPromise;
+    Assert.equal(gBrowser.currentURI.spec, "http://example.com/");
+  });
+  await ext.unload();
+});
+
+// Loads an extension with a main button URL and clicks the main button.
+add_task(async function testOnResultPicked_mainButton_url_mouse() {
+  let ext = await loadExtension({ buttonUrl: "http://example.com/" });
+  await BrowserTestUtils.withNewTab("about:blank", async () => {
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      waitForFocus,
+      value: "test",
+    });
+    let mainButton = document.querySelector(
+      "#urlbarView-row-0 .urlbarView-tip-button"
+    );
+    Assert.ok(mainButton);
+    let loadedPromise = BrowserTestUtils.browserLoaded(
+      gBrowser.selectedBrowser
+    );
+    ext.onMessage("onResultPicked received", () => {
+      Assert.ok(false, "onResultPicked should not be called");
+    });
+    EventUtils.synthesizeMouseAtCenter(mainButton, {});
+    await loadedPromise;
+    Assert.equal(gBrowser.currentURI.spec, "http://example.com/");
+  });
+  await ext.unload();
+});
+
+// Loads an extension without a help button URL and presses enter on the help
+// button.
+add_task(async function testOnResultPicked_helpButton_noURL_enter() {
+  let ext = await loadExtension();
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "test",
+  });
+  EventUtils.synthesizeKey("KEY_ArrowDown", { repeat: 2 });
+  EventUtils.synthesizeKey("KEY_Enter");
+  let details = await ext.awaitMessage("onResultPicked received");
+  Assert.deepEqual(details, { helpPicked: true });
+  await ext.unload();
+});
+
+// Loads an extension without a help button URL and clicks the help button.
+add_task(async function testOnResultPicked_helpButton_noURL_mouse() {
+  let ext = await loadExtension();
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    waitForFocus,
+    value: "test",
+  });
+  let helpButton = document.querySelector(
+    "#urlbarView-row-0 .urlbarView-tip-help"
+  );
+  Assert.ok(helpButton);
+  EventUtils.synthesizeMouseAtCenter(helpButton, {});
+  let details = await ext.awaitMessage("onResultPicked received");
+  Assert.deepEqual(details, { helpPicked: true });
+  await ext.unload();
+});
+
+// Loads an extension with a help button URL and presses enter on the help
+// button.
+add_task(async function testOnResultPicked_helpButton_url_enter() {
+  let ext = await loadExtension({ helpUrl: "http://example.com/" });
+  await BrowserTestUtils.withNewTab("about:blank", async () => {
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      waitForFocus,
+      value: "test",
+    });
+    let loadedPromise = BrowserTestUtils.browserLoaded(
+      gBrowser.selectedBrowser
+    );
+    ext.onMessage("onResultPicked received", () => {
+      Assert.ok(false, "onResultPicked should not be called");
+    });
+    EventUtils.synthesizeKey("KEY_ArrowDown", { repeat: 2 });
+    EventUtils.synthesizeKey("KEY_Enter");
+    await loadedPromise;
+    Assert.equal(gBrowser.currentURI.spec, "http://example.com/");
+  });
+  await ext.unload();
+});
+
+// Loads an extension with a help button URL and clicks the help button.
+add_task(async function testOnResultPicked_helpButton_url_mouse() {
+  let ext = await loadExtension({ helpUrl: "http://example.com/" });
+  await BrowserTestUtils.withNewTab("about:blank", async () => {
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      waitForFocus,
+      value: "test",
+    });
+    let helpButton = document.querySelector(
+      "#urlbarView-row-0 .urlbarView-tip-help"
+    );
+    Assert.ok(helpButton);
+    let loadedPromise = BrowserTestUtils.browserLoaded(
+      gBrowser.selectedBrowser
+    );
+    ext.onMessage("onResultPicked received", () => {
+      Assert.ok(false, "onResultPicked should not be called");
+    });
+    EventUtils.synthesizeMouseAtCenter(helpButton, {});
+    await loadedPromise;
+    Assert.equal(gBrowser.currentURI.spec, "http://example.com/");
+  });
+  await ext.unload();
+});
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -648,32 +648,30 @@ class UrlbarInput {
           isSuggestion: !!result.payload.suggestion,
           alias: result.payload.keyword,
         };
         const engine = Services.search.getEngineByName(result.payload.engine);
         this._recordSearch(engine, event, actionDetails);
         break;
       }
       case UrlbarUtils.RESULT_TYPE.TIP: {
-        if (element.classList.contains("urlbarView-tip-help")) {
+        let helpPicked = element.classList.contains("urlbarView-tip-help");
+        if (helpPicked) {
           url = result.payload.helpUrl;
         }
-
         if (!url) {
           this.handleRevert();
           this.controller.engagementEvent.record(event, {
             numChars: this._lastSearchString.length,
             selIndex,
             selType: "tip",
           });
-
-          // TODO: Call out to UrlbarProvider.pickElement as part of bug 1578584.
+          result.provider.pickResult(result, { helpPicked });
           return;
         }
-
         break;
       }
       case UrlbarUtils.RESULT_TYPE.OMNIBOX: {
         this.controller.engagementEvent.record(event, {
           numChars: this._lastSearchString.length,
           selIndex,
           selType: "extension",
         });
--- a/browser/components/urlbar/UrlbarProviderExtension.jsm
+++ b/browser/components/urlbar/UrlbarProviderExtension.jsm
@@ -209,33 +209,47 @@ class UrlbarProviderExtension extends Ur
    * @param {UrlbarQueryContext} context
    *   The query context.
    */
   cancelQuery(context) {
     this._notifyListener("queryCanceled", context);
   }
 
   /**
+   * This method is called when a result from the provider without a URL is
+   * picked, but currently only for tip results.  The provider should handle the
+   * pick.
+   *
+   * @param {UrlbarResult} result
+   *   The result that was picked.
+   * @param {object} details
+   *   Details about the pick, depending on the result type.
+   */
+  pickResult(result, details) {
+    this._notifyListener("resultPicked", result.payload, details);
+  }
+
+  /**
    * Calls a listener function set by the extension API implementation, if any.
    *
    * @param {string} eventName
    *   The name of the listener to call (i.e., the name of the event to fire).
-   * @param {UrlbarQueryContext} context
-   *   The query context relevant to the event.
+   * @param {arguments} args
+   *   The arguments to pass to the listener.
    * @returns {*}
    *   The value returned by the listener function, if any.
    */
-  async _notifyListener(eventName, context) {
+  async _notifyListener(eventName, ...args) {
     let listener = this._eventListeners.get(eventName);
     if (!listener) {
       return undefined;
     }
     let result;
     try {
-      result = listener(context);
+      result = listener(...args);
     } catch (error) {
       Cu.reportError(error);
       return undefined;
     }
     if (result.catch) {
       // The result is a promise, so wait for it to be resolved.  Set up a timer
       // so that we're not stuck waiting forever.
       let timer = new SkippableTimer({
--- a/browser/components/urlbar/UrlbarProvidersManager.jsm
+++ b/browser/components/urlbar/UrlbarProvidersManager.jsm
@@ -362,16 +362,17 @@ class Query {
       match.payload.url &&
       match.payload.url.startsWith("javascript:") &&
       !this.context.searchString.startsWith("javascript:") &&
       UrlbarPrefs.get("filter.javascript")
     ) {
       return;
     }
 
+    match.provider = provider;
     this.context.results.push(match);
 
     let notifyResults = () => {
       if (this._chunkTimer) {
         this._chunkTimer.cancel().catch(Cu.reportError);
         delete this._chunkTimer;
       }
       this.muxer.sort(this.context);
--- a/browser/components/urlbar/UrlbarUtils.jsm
+++ b/browser/components/urlbar/UrlbarUtils.jsm
@@ -602,16 +602,17 @@ class UrlbarMuxer {
   /**
    * Unique name for the muxer, used by the context to sort results.
    * Not using a unique name will cause the newest registration to win.
    * @abstract
    */
   get name() {
     return "UrlbarMuxerBase";
   }
+
   /**
    * Sorts queryContext results in-place.
    * @param {UrlbarQueryContext} queryContext the context to sort results for.
    * @abstract
    */
   sort(queryContext) {
     throw new Error("Trying to access the base class, must be overridden");
   }
@@ -625,66 +626,84 @@ class UrlbarProvider {
   /**
    * Unique name for the provider, used by the context to filter on providers.
    * Not using a unique name will cause the newest registration to win.
    * @abstract
    */
   get name() {
     return "UrlbarProviderBase";
   }
+
   /**
    * The type of the provider, must be one of UrlbarUtils.PROVIDER_TYPE.
    * @abstract
    */
   get type() {
     throw new Error("Trying to access the base class, must be overridden");
   }
+
   /**
    * Whether this provider should be invoked for the given context.
    * If this method returns false, the providers manager won't start a query
    * with this provider, to save on resources.
    * @param {UrlbarQueryContext} queryContext The query context object
    * @returns {boolean} Whether this provider should be invoked for the search.
    * @abstract
    */
   isActive(queryContext) {
     throw new Error("Trying to access the base class, must be overridden");
   }
+
   /**
    * Whether this provider wants to restrict results to just itself.
    * Other providers won't be invoked, unless this provider doesn't
    * support the current query.
    * @param {UrlbarQueryContext} queryContext The query context object
    * @returns {boolean} Whether this provider wants to restrict results.
    * @abstract
    */
   isRestricting(queryContext) {
     throw new Error("Trying to access the base class, must be overridden");
   }
+
   /**
    * Starts querying.
    * @param {UrlbarQueryContext} queryContext The query context object
    * @param {function} addCallback Callback invoked by the provider to add a new
    *        result. A UrlbarResult should be passed to it.
    * @note Extended classes should return a Promise resolved when the provider
    *       is done searching AND returning results.
    * @abstract
    */
   startQuery(queryContext, addCallback) {
     throw new Error("Trying to access the base class, must be overridden");
   }
+
   /**
    * Cancels a running query,
    * @param {UrlbarQueryContext} queryContext the query context object to cancel
    *        query for.
    * @abstract
    */
   cancelQuery(queryContext) {
     throw new Error("Trying to access the base class, must be overridden");
   }
+
+  /**
+   * Called when a result from the provider without a URL is picked, but
+   * currently only for tip results.  The provider should handle the pick.
+   * @param {UrlbarResult} result
+   *   The result that was picked.
+   * @param {object} details
+   *   Details about the pick, depending on the result type.
+   * @abstract
+   */
+  pickResult(result, details) {
+    throw new Error("Trying to access the base class, must be overridden");
+  }
 }
 
 /**
  * Class used to create a timer that can be manually fired, to immediately
  * invoke the callback, or canceled, as necessary.
  * Examples:
  *   let timer = new SkippableTimer();
  *   // Invokes the callback immediately without waiting for the delay.