Bug 1495614 - Don't highlight the search engine alias when the urlbar input doesn't match the first heuristic result r=mak
authorDrew Willcoxon <adw@mozilla.com>
Fri, 12 Oct 2018 13:45:37 +0000
changeset 489381 8411d22c2440f4b52e0b835d2a223715d92adf65
parent 489380 8aa6552b8f99bfd3358ae3306625461e6f4678e5
child 489382 7264528833241157fa745cc0036baf1b1a24ab60
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersmak
bugs1495614
milestone64.0a1
Bug 1495614 - Don't highlight the search engine alias when the urlbar input doesn't match the first heuristic result r=mak Differential Revision: https://phabricator.services.mozilla.com/D8479
browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
browser/components/urlbar/UrlbarValueFormatter.jsm
--- a/browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
+++ b/browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
@@ -59,16 +59,60 @@ add_task(async function aliasCase() {
   await promiseSearchComplete();
   await waitForAutocompleteResultAt(0);
   assertAlias(true, alias);
 
   EventUtils.synthesizeKey("KEY_Escape");
   await promisePopupHidden(gURLBar.popup);
 });
 
+add_task(async function inputDoesntMatchHeuristicResult() {
+  // Do a search using the alias.
+  let searchString = `${ALIAS} aaa`;
+  gURLBar.search(searchString);
+  await promiseSearchComplete();
+  await waitForAutocompleteResultAt(0);
+  assertAlias(true);
+
+  // Hide the popup.
+  EventUtils.synthesizeKey("KEY_Escape");
+  await promisePopupHidden(gURLBar.popup);
+
+  // Manually set the urlbar value to a string that contains the alias at the
+  // beginning but is not the same as the search string.
+  let value = `${ALIAS} xxx`;
+  gURLBar.value = `${ALIAS} xxx`;
+
+  // The alias substring should not be highlighted.
+  Assert.equal(gURLBar.value, value);
+  Assert.ok(gURLBar.value.includes(ALIAS));
+  assertHighlighted(false, ALIAS);
+
+  // Do another search using the alias.
+  searchString = `${ALIAS} bbb`;
+  gURLBar.search(searchString);
+  await promiseSearchComplete();
+  await waitForAutocompleteResultAt(0);
+  assertAlias(true);
+
+  // Hide the popup.
+  EventUtils.synthesizeKey("KEY_Escape");
+  await promisePopupHidden(gURLBar.popup);
+
+  // Manually set the urlbar value to a string that contains the alias, but not
+  // at the beginning and is not the same as the search string.
+  value = `bbb ${ALIAS}`;
+  gURLBar.value = `bbb ${ALIAS}`;
+
+  // The alias substring should not be highlighted.
+  Assert.equal(gURLBar.value, value);
+  Assert.ok(gURLBar.value.includes(ALIAS));
+  assertHighlighted(false, ALIAS);
+});
+
 
 async function doTest(revertBetweenSteps) {
   // "@tes" -- not an alias, no highlight
   gURLBar.search(ALIAS.substr(0, ALIAS.length - 1));
   await promiseSearchComplete();
   await waitForAutocompleteResultAt(0);
   assertAlias(false);
 
--- a/browser/components/urlbar/UrlbarValueFormatter.jsm
+++ b/browser/components/urlbar/UrlbarValueFormatter.jsm
@@ -267,43 +267,61 @@ class UrlbarValueFormatter {
    * @returns {boolean}
    *   True if formatting was applied and false if not.
    */
   _formatSearchAlias() {
     if (!UrlbarPrefs.get("formatting.enabled")) {
       return false;
     }
 
-    // There can only be an alias to highlight if the heuristic result is
-    // an alias searchengine result and it's either currently selected or
-    // was selected when the popup was closed.  We also need to check
-    // whether a one-off search button is selected because in that case
-    // there won't be a selection but the alias should not be highlighted.
     let popup = this.urlbarInput.popup;
     if (!popup) {
       // TODO: make this work with UrlbarView
       return false;
     }
+
+    // There can only be an alias to highlight if the heuristic result is
+    // an alias searchengine result and it's either currently selected or
+    // was selected when the popup was closed.  We also need to check
+    // whether a one-off search button is selected because in that case
+    // there won't be a selection but the alias should not be highlighted.
+    if ((popup.selectedIndex < 0 &&
+         popup._previousSelectedIndex != 0) ||
+        popup.selectedIndex > 0 ||
+        popup.oneOffSearchButtons.selectedButton) {
+      return false;
+    }
     let heuristicItem = popup.richlistbox.children[0] || null;
-    let alias =
-      (popup.selectedIndex == 0 ||
-       (popup.selectedIndex < 0 &&
-        popup._previousSelectedIndex == 0)) &&
-      !popup.oneOffSearchButtons.selectedButton &&
-      heuristicItem &&
-      heuristicItem.getAttribute("actiontype") == "searchengine" &&
-      this.urlbarInput._parseActionUrl(heuristicItem.getAttribute("url")).params.alias;
+    if (!heuristicItem ||
+        heuristicItem.getAttribute("actiontype") != "searchengine") {
+      return false;
+    }
+    let url = heuristicItem.getAttribute("url");
+    let action = this.urlbarInput._parseActionUrl(url);
+    if (!action) {
+      return false;
+    }
+    let alias = action.params.alias || null;
     if (!alias) {
       return false;
     }
 
     let editor = this.urlbarInput.editor;
     let textNode = editor.rootElement.firstChild;
     let value = textNode.textContent;
 
+    // Make sure the heuristic result's input matches the current urlbar input
+    // because the urlbar input can change without the popup results changing.
+    // Most notably that happens when the user performs a search using an alias:
+    // The popup closes, the search results page is loaded, and the urlbar value
+    // is set to the URL of the page.
+    if (decodeURIComponent(action.params.input) != value) {
+      return false;
+    }
+
     let index = value.indexOf(alias);
     if (index < 0) {
       return false;
     }
 
     // We abuse the SELECTION_FIND selection type to do our highlighting.
     // It's the only type that works with Selection.setColors().
     let selection = editor.selectionController.getSelection(