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 499368 8411d22c2440f4b52e0b835d2a223715d92adf65
parent 499367 8aa6552b8f99bfd3358ae3306625461e6f4678e5
child 499369 7264528833241157fa745cc0036baf1b1a24ab60
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1495614
milestone64.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 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(