Bug 1594062 - urlbar part - When selecting a one-off button with the keyboard, show action text on a search heuristic result. r=Standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 19 Nov 2019 15:53:59 +0000
changeset 502628 44bb8023055ce5553735b036dd60c09429907dae
parent 502627 7f80960886b9a5fafee81e6b95fe87ad5ace04c6
child 502629 5b05b982134618f31c0f3b3a21ad33dbb95bec99
push id36819
push userffxbld
push dateTue, 19 Nov 2019 21:20:10 +0000
treeherdermozilla-central@5fd5626f1b89 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1594062
milestone72.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 1594062 - urlbar part - When selecting a one-off button with the keyboard, show action text on a search heuristic result. r=Standard8 When a one-off button is the only selection, and the heuristic result is the a search, forcibly show its action text, so the search engine name is visible. This doesn't handle cases where the heuristic results are not searches, because it would be a lot more complex and involve heavy design changes. Those cases should be less common, representing searches for urls or non-search strings on other engines. Differential Revision: https://phabricator.services.mozilla.com/D53785
browser/components/urlbar/UrlbarView.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
browser/components/urlbar/tests/browser/browser_urlbarOneOffs_searchSuggestions.js
browser/themes/shared/urlbar-autocomplete.inc.css
--- a/browser/components/urlbar/UrlbarView.jsm
+++ b/browser/components/urlbar/UrlbarView.jsm
@@ -1337,16 +1337,23 @@ class UrlbarView {
           result.payload.originalEngine = result.payload.engine;
         }
         result.payload.engine = engine.name;
       } else if (result.payload.originalEngine) {
         result.payload.engine = result.payload.originalEngine;
         delete result.payload.originalEngine;
       }
       let item = this._rows.children[i];
+      // If a one-off button is the only selection, force the heuristic result
+      // to show its action text, so the engine name is visible.
+      if (result.heuristic && engine && !this.selectedElement) {
+        item.setAttribute("show-action-text", "true");
+      } else {
+        item.removeAttribute("show-action-text");
+      }
       if (!result.payload.inPrivateWindow) {
         let action = item.querySelector(".urlbarView-action");
         action.textContent = UrlbarUtils.strings.formatStringFromName(
           "searchWithEngine",
           [(engine && engine.name) || result.payload.engine]
         );
       }
       // If we just changed the engine from the original engine and it had an
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -172,16 +172,17 @@ tags = clipboard
 [browser_urlbarOneOffs_contextMenu.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_urlbarOneOffs_searchSuggestions.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
+  searchSuggestionEngine2.xml
 [browser_urlbarOneOffs_settings.js]
 [browser_urlbarOneOffs.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_urlbarPlaceholder.js]
 support-files =
   searchSuggestionEngine.xml
--- a/browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
+++ b/browser/components/urlbar/tests/browser/browser_urlbarOneOffs.js
@@ -67,63 +67,94 @@ add_task(async function history() {
 // Keys up and down through the non-history panel, i.e., the panel that's shown
 // when you type something in the textbox.
 add_task(async function() {
   // Use a typed value that returns the visits added above but that doesn't
   // trigger autofill since that would complicate the test.
   let typedValue = "browser_urlbarOneOffs";
   await promiseAutocompleteResultPopup(typedValue, window, true);
   await waitForAutocompleteResultAt(gMaxResults - 1);
+  let heuristicResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
   assertState(0, -1, typedValue);
 
   // Key down through each result.  The first result is already selected, which
   // is why gMaxResults - 1 is the correct number of times to do this.
   for (let i = 0; i < gMaxResults - 1; i++) {
     EventUtils.synthesizeKey("KEY_ArrowDown");
     // i starts at zero so that the textValue passed to assertState is correct.
     // But that means that i + 1 is the expected selected index, since initially
     // (when this loop starts) the first result is selected.
     assertState(
       i + 1,
       -1,
       "example.com/browser_urlbarOneOffs.js/?" + (gMaxResults - i - 1)
     );
+    Assert.ok(
+      !BrowserTestUtils.is_visible(heuristicResult.element.action),
+      "The heuristic action should not be visible"
+    );
   }
 
   // Key down through each one-off.
   let numButtons = oneOffSearchButtons.getSelectableButtons(true).length;
   for (let i = 0; i < numButtons; i++) {
     EventUtils.synthesizeKey("KEY_ArrowDown");
     assertState(-1, i, typedValue);
+    Assert.equal(
+      BrowserTestUtils.is_visible(heuristicResult.element.action),
+      !oneOffSearchButtons.selectedButton.classList.contains(
+        "search-setting-button-compact"
+      ),
+      "The heuristic action should be visible when a one-off button is selected"
+    );
   }
 
   // Key down once more.  The selection should wrap around to the first result.
   EventUtils.synthesizeKey("KEY_ArrowDown");
   assertState(0, -1, typedValue);
+  Assert.ok(
+    BrowserTestUtils.is_visible(heuristicResult.element.action),
+    "The heuristic action should be visible"
+  );
 
   // Now key up.  The selection should wrap back around to the one-offs.  Key
   // up through all the one-offs.
   for (let i = numButtons - 1; i >= 0; i--) {
     EventUtils.synthesizeKey("KEY_ArrowUp");
     assertState(-1, i, typedValue);
+    Assert.equal(
+      BrowserTestUtils.is_visible(heuristicResult.element.action),
+      !oneOffSearchButtons.selectedButton.classList.contains(
+        "search-setting-button-compact"
+      ),
+      "The heuristic action should be visible when a one-off button is selected"
+    );
   }
 
   // Key up through each non-heuristic result.
   for (let i = gMaxResults - 2; i >= 0; i--) {
     EventUtils.synthesizeKey("KEY_ArrowUp");
     assertState(
       i + 1,
       -1,
       "example.com/browser_urlbarOneOffs.js/?" + (gMaxResults - i - 1)
     );
+    Assert.ok(
+      !BrowserTestUtils.is_visible(heuristicResult.element.action),
+      "The heuristic action should not be visible"
+    );
   }
 
   // Key up once more.  The heuristic result should be selected.
   EventUtils.synthesizeKey("KEY_ArrowUp");
   assertState(0, -1, typedValue);
+  Assert.ok(
+    BrowserTestUtils.is_visible(heuristicResult.element.action),
+    "The heuristic action should be visible"
+  );
 
   await hidePopup();
 });
 
 // Checks that "Search with Current Search Engine" items are updated to "Search
 // with One-Off Engine" when a one-off is selected.
 add_task(async function searchWith() {
   let typedValue = "foo";
--- a/browser/components/urlbar/tests/browser/browser_urlbarOneOffs_searchSuggestions.js
+++ b/browser/components/urlbar/tests/browser/browser_urlbarOneOffs_searchSuggestions.js
@@ -67,16 +67,21 @@ async function withSecondSuggestion(test
 
 // Presses the Return key when a one-off is selected after selecting a search
 // suggestion.
 add_task(async function test_returnAfterSuggestion() {
   await withSecondSuggestion(async index => {
     // Alt+Down to select the first one-off.
     EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true });
     assertState(index, 0, "foobar");
+    let heuristicResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+    Assert.ok(
+      !BrowserTestUtils.is_visible(heuristicResult.element.action),
+      "The heuristic action should not be visible"
+    );
 
     let resultsPromise = BrowserTestUtils.browserLoaded(
       gBrowser.selectedBrowser,
       false,
       `http://mochi.test:8888/?terms=foobar`
     );
     EventUtils.synthesizeKey("KEY_Enter");
     await resultsPromise;
@@ -142,16 +147,22 @@ add_task(async function test_selectOneOf
     let index = await UrlbarTestUtils.promiseSuggestionsPresent(window);
     assertState(0, -1, typedValue);
 
     // Select a non-default one-off engine.
     EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true });
     EventUtils.synthesizeKey("KEY_ArrowDown", { altKey: true });
     assertState(0, 1, "foo");
 
+    let heuristicResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+    Assert.ok(
+      BrowserTestUtils.is_visible(heuristicResult.element.action),
+      "The heuristic action should be visible because the result is selected"
+    );
+
     // Now click the second suggestion.
     await withHttpServer(serverInfo, async () => {
       let result = await UrlbarTestUtils.getDetailsOfResultAt(
         window,
         index + 1
       );
 
       let resultsPromise = BrowserTestUtils.browserLoaded(
--- a/browser/themes/shared/urlbar-autocomplete.inc.css
+++ b/browser/themes/shared/urlbar-autocomplete.inc.css
@@ -359,18 +359,18 @@
 .urlbarView-row[selected] > .urlbarView-row-inner > .urlbarView-title-separator::before,
 .urlbarView-row[selected] > .urlbarView-row-inner > .urlbarView-secondary {
   color: inherit;
 }
 
 .urlbarView-row[type=remotetab][selected] > .urlbarView-row-inner > .urlbarView-action,
 .urlbarView-row[type=remotetab]:hover > .urlbarView-row-inner > .urlbarView-action,
 .urlbarView-row[type=remotetab]:not([selected]):not(:hover) > .urlbarView-row-inner > .urlbarView-url,
-.urlbarView-row[type=search]:not([selected]):not(:hover) > .urlbarView-row-inner > .urlbarView-title:not(:empty) ~ .urlbarView-action,
-.urlbarView-row[type=search]:not([selected]):not(:hover) > .urlbarView-row-inner > .urlbarView-title-separator,
+.urlbarView-row[type=search]:not([selected]):not(:hover):not([show-action-text]) > .urlbarView-row-inner > .urlbarView-title:not(:empty) ~ .urlbarView-action,
+.urlbarView-row[type=search]:not([selected]):not(:hover):not([show-action-text]) > .urlbarView-row-inner > .urlbarView-title-separator,
 .urlbarView[actionoverride] .urlbarView-row[type=switchtab] > .urlbarView-row-inner > .urlbarView-action,
 .urlbarView:not([actionoverride]) .urlbarView-row[type=switchtab] > .urlbarView-row-inner > .urlbarView-url {
   /* Use visibility:collapse instead of display:none since the latter can
      confuse the overflow state of title and url elements when their content
      changes while they're hidden. */
   visibility: collapse;
 }