Bug 1594622 - Quantumbar: Remove the context.preselected property and rely on result.heuristic instead r=mak

Please see bug 1594622 for a description.

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

--HG--
extra : moz-landing-system : lando

browser/components/extensions/test/browser/browser_ext_urlbar.js

@@ -82,7 +82,6 @@ add_task(async function tip_onResultPicked_mainButton_noURL_enter() {
     waitForFocus,
     value: "test",
   });
-  EventUtils.synthesizeKey("KEY_ArrowDown");
   EventUtils.synthesizeKey("KEY_Enter");
   await ext.awaitMessage("onResultPicked received");
   await ext.unload();
@@ -121,7 +120,6 @@ add_task(async function tip_onResultPicked_mainButton_url_enter() {
     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/");
@@ -171,7 +169,7 @@ add_task(async function tip_onResultPicked_helpButton_url_enter() {
     ext.onMessage("onResultPicked received", () => {
       Assert.ok(false, "onResultPicked should not be called");
     });
-    EventUtils.synthesizeKey("KEY_ArrowDown", { repeat: 2 });
+    EventUtils.synthesizeKey("KEY_ArrowDown");
     EventUtils.synthesizeKey("KEY_Enter");
     await loadedPromise;
     Assert.equal(gBrowser.currentURI.spec, "http://example.com/");

browser/components/extensions/test/xpcshell/test_ext_urlbar.js

@@ -820,6 +820,129 @@ add_task(async function test_activeInactiveAndRestrictingProviders() {
   await ext.unload();
 });
 
+// Adds a restricting provider that returns a heuristic result.  The actual
+// result created from the extension's result should be a heuristic.
+add_task(async function test_heuristicRestricting() {
+  let ext = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["urlbar"],
+    },
+    isPrivileged: true,
+    incognitoOverride: "spanning",
+    background() {
+      browser.urlbar.onBehaviorRequested.addListener(query => {
+        return "restricting";
+      }, "test");
+      browser.urlbar.onResultsRequested.addListener(query => {
+        return [
+          {
+            type: "url",
+            source: "history",
+            heuristic: true,
+            payload: {
+              title: "Test result",
+              url: "http://example.com/",
+            },
+          },
+        ];
+      }, "test");
+    },
+  });
+  await ext.startup();
+
+  // Check the provider.
+  let provider = UrlbarProvidersManager.getProvider("test");
+  Assert.ok(provider);
+
+  // Run a query.
+  let context = new UrlbarQueryContext({
+    allowAutofill: false,
+    isPrivate: false,
+    maxResults: 10,
+    searchString: "test",
+  });
+  let controller = new UrlbarController({
+    browserWindow: {
+      location: {
+        href: AppConstants.BROWSER_CHROME_URL,
+      },
+    },
+  });
+  await controller.startQuery(context);
+
+  // Check the results.
+  Assert.equal(context.results.length, 1);
+  Assert.ok(context.results[0].heuristic);
+
+  await ext.unload();
+});
+
+// Adds a non-restricting provider that returns a heuristic result.  The actual
+// result created from the extension's result should *not* be a heuristic, and
+// the usual UnifiedComplete heuristic should be present.
+add_task(async function test_heuristicNonRestricting() {
+  let ext = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["urlbar"],
+    },
+    isPrivileged: true,
+    incognitoOverride: "spanning",
+    background() {
+      browser.urlbar.onBehaviorRequested.addListener(query => {
+        return "active";
+      }, "test");
+      browser.urlbar.onResultsRequested.addListener(query => {
+        return [
+          {
+            type: "url",
+            source: "history",
+            heuristic: true,
+            payload: {
+              title: "Test result",
+              url: "http://example.com/",
+            },
+          },
+        ];
+      }, "test");
+    },
+  });
+  await ext.startup();
+
+  // Check the provider.
+  let provider = UrlbarProvidersManager.getProvider("test");
+  Assert.ok(provider);
+
+  // Run a query.
+  let context = new UrlbarQueryContext({
+    allowAutofill: false,
+    isPrivate: false,
+    maxResults: 10,
+    searchString: "test",
+  });
+  let controller = new UrlbarController({
+    browserWindow: {
+      location: {
+        href: AppConstants.BROWSER_CHROME_URL,
+      },
+    },
+  });
+  await controller.startQuery(context);
+
+  // Check the results.  The first result should be UnifiedComplete's heuristic.
+  let firstResult = context.results[0];
+  Assert.ok(firstResult.heuristic);
+  Assert.equal(firstResult.type, UrlbarUtils.RESULT_TYPE.SEARCH);
+  Assert.equal(firstResult.source, UrlbarUtils.RESULT_SOURCE.SEARCH);
+  Assert.equal(firstResult.payload.engine, "Test engine");
+
+  // The extension result should be present but not the heuristic.
+  let result = context.results.find(r => r.title == "Test result");
+  Assert.ok(result);
+  Assert.ok(!result.heuristic);
+
+  await ext.unload();
+});
+
 // Adds an active provider that doesn't have a listener for onResultsRequested.
 // No results should be added.
 add_task(async function test_onResultsRequestedNotImplemented() {

browser/components/urlbar/UrlbarController.jsm

@@ -404,7 +404,7 @@ class UrlbarController {
       case "resultsadded": {
         // We should connect to an heuristic result, if it exists.
         if (
-          (result == context.results[0] && context.preselected) ||
+          (result == context.results[0] && result.heuristic) ||
           result.autofill
         ) {
           if (result.type == UrlbarUtils.RESULT_TYPE.SEARCH) {

browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm

@@ -75,13 +75,11 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
     if (!context.results.length) {
       return;
     }
-    // Look for an heuristic result.  If it's a preselected search result, use
-    // search buckets, otherwise use normal buckets.
+    // Look for an heuristic result.  If it's a search result, use search
+    // buckets, otherwise use normal buckets.
     let heuristicResult = context.results.find(r => r.heuristic);
     let buckets =
-      context.preselected &&
-      heuristicResult &&
-      heuristicResult.type == UrlbarUtils.RESULT_TYPE.SEARCH
+      heuristicResult && heuristicResult.type == UrlbarUtils.RESULT_TYPE.SEARCH
         ? UrlbarPrefs.get("matchBucketsSearch")
         : UrlbarPrefs.get("matchBuckets");
     logger.debug(`Buckets: ${buckets}`);
@@ -108,8 +106,7 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
 
         if (
           group == UrlbarUtils.RESULT_GROUP.HEURISTIC &&
-          result == heuristicResult &&
-          context.preselected
+          result == heuristicResult
         ) {
           // Handle the heuristic result.
           sortedResults.unshift(result);

browser/components/urlbar/UrlbarProviderExtension.jsm

@@ -318,6 +318,13 @@ class UrlbarProviderExtension extends UrlbarProvider {
         extResult.payload || {}
       )
     );
+    if (extResult.heuristic && this.behavior == "restricting") {
+      // The muxer chooses the final heuristic result by taking the first one
+      // that claims to be the heuristic.  We don't want extensions to clobber
+      // UnifiedComplete's heuristic, so we allow this only if the provider is
+      // restricting.
+      result.heuristic = extResult.heuristic;
+    }
     if (extResult.suggestedIndex !== undefined) {
       result.suggestedIndex = extResult.suggestedIndex;
     }

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -218,28 +218,24 @@ function convertResultToMatches(context, result, urls) {
     if (!match) {
       continue;
     }
-    // Manage autofill and preselected properties for the first match.
-    if (isHeuristic) {
-      if (style.includes("autofill") && result.defaultIndex == 0) {
-        let autofillValue = result.getValueAt(i);
-        if (
-          autofillValue
-            .toLocaleLowerCase()
-            .startsWith(context.searchString.toLocaleLowerCase())
-        ) {
-          match.autofill = {
-            value:
-              context.searchString +
-              autofillValue.substring(context.searchString.length),
-            selectionStart: context.searchString.length,
-            selectionEnd: autofillValue.length,
-          };
-        }
+    // Manage autofill for the first match.
+    if (isHeuristic && style.includes("autofill") && result.defaultIndex == 0) {
+      let autofillValue = result.getValueAt(i);
+      if (
+        autofillValue
+          .toLocaleLowerCase()
+          .startsWith(context.searchString.toLocaleLowerCase())
+      ) {
+        match.autofill = {
+          value:
+            context.searchString +
+            autofillValue.substring(context.searchString.length),
+          selectionStart: context.searchString.length,
+          selectionEnd: autofillValue.length,
+        };
       }
-
-      context.preselected = true;
-      match.heuristic = true;
     }
+    match.heuristic = isHeuristic;
     matches.push(match);
   }
   return { matches, done };

browser/components/urlbar/UrlbarView.jsm

@@ -442,10 +442,12 @@ class UrlbarView {
     }
     this._updateResults(queryContext);
 
-    let isFirstPreselectedResult = false;
+    let isHeuristicResult = false;
     if (queryContext.lastResultCount == 0) {
-      if (queryContext.preselected) {
-        isFirstPreselectedResult = true;
+      let firstResult = queryContext.results[0];
+
+      if (firstResult.heuristic) {
+        isHeuristicResult = true;
         this._selectElement(this._getFirstSelectableElement(), {
           updateInput: false,
           setAccessibleFocus: this.controller._userSelectionBehavior == "arrow",
@@ -469,15 +471,15 @@ class UrlbarView {
 
       // The input field applies autofill on input, without waiting for results.
       // Once we get results, we can ask it to correct wrong predictions.
-      this.input.maybeClearAutofillPlaceholder(queryContext.results[0]);
+      this.input.maybeClearAutofillPlaceholder(firstResult);
     }
 
     this._openPanel();
 
-    if (isFirstPreselectedResult) {
-      // The first, preselected result may be a search alias result, so apply
-      // formatting if necessary.  Conversely, the first result of the previous
-      // query may have been an alias, so remove formatting if necessary.
+    if (isHeuristicResult) {
+      // The heuristic result may be a search alias result, so apply formatting
+      // if necessary.  Conversely, the heuristic result of the previous query
+      // may have been an alias, so remove formatting if necessary.
       this.input.formatValue();
     }
   }

browser/components/urlbar/docs/overview.rst

@@ -46,7 +46,6 @@ It is augmented as it progresses through the system, with various information:
              // RESULT_SOURCE.*, that can be returned by the model.
 
     // Properties added by the Model.
-    preselected; // {boolean} whether the first result should be preselected.
     results; // {array} list of UrlbarResult objects.
     tokens; // {array} tokens extracted from the searchString, each token is an
             // object in the form {type, value, lowerCaseValue}.

browser/components/urlbar/tests/unit/test_muxer.js

@@ -115,7 +115,6 @@ add_task(async function test_preselectedHeuristic_singleProvider() {
   let providerName = registerBasicTestProvider(matches);
   let context = createContext(undefined, {
     providers: [providerName],
-    preselected: true,
   });
   let controller = new UrlbarController({
     browserWindow: {
@@ -173,7 +172,6 @@ add_task(async function test_preselectedHeuristic_multiProviders() {
 
   let context = createContext(undefined, {
     providers: [provider1Name, provider2Name],
-    preselected: true,
   });
   let controller = new UrlbarController({
     browserWindow: {