Bug 1678765 - Fix broken "Search with" action text for heuristic results that are restyled to look like search results. r=harry

This was a little harder than it seemed like it should be because we show/hide
the title separator in two places, in `UrlbarView.updateRow` and in the CSS.
This patch gets rid of the show/hide in `updateRow`, so now we show/hide only in
the CSS. The decision to show/hide in `updateRow` was based on whether the
result has action text (`actionSetter`) or is a URL (`setURL`). We already had a
`has-url` attribute that corresponds directly to `setURL`, so adding a similar
`has-action` attribute that corresponds to `actionSetter` lets us show/hide only
in the CSS.

The second part of this patch is to actually fix the bug. For that, the existing
`show-action-text` attribute does what we want in the CSS; the only problem is
that we currently set it only when there's no selected row, but we need to also
set it when there's no selected row but a one-off is selected. We have a similar
block -- the one with a comment about restyling the heuristic to look like a
search result -- so I removed the block that currently sets/removes the
attribute and moved the set/removal there. I also renamed the attribute
`restyled-search` to better semantically describe what's happening, but if you
disagree I can restore the old name.

Depends on D97843

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

browser/components/urlbar/UrlbarView.jsm

@@ -1365,12 +1365,14 @@ class UrlbarView {
     if (actionSetter) {
       actionSetter();
       item._originalActionSetter = actionSetter;
+      item.setAttribute("has-action", "true");
     } else {
       item._originalActionSetter = () => {
         action.removeAttribute("data-l10n-id");
         action.textContent = "";
       };
       item._originalActionSetter();
+      item.removeAttribute("has-action");
     }
 
     if (!title.hasAttribute("isurl")) {
@@ -1378,8 +1380,6 @@ class UrlbarView {
     } else {
       title.removeAttribute("dir");
     }
-
-    item._elements.get("titleSeparator").hidden = !actionSetter && !setURL;
   }
 
   /**
@@ -1960,18 +1960,6 @@ class UrlbarView {
       let favicon = item.querySelector(".urlbarView-favicon");
       let title = item.querySelector(".urlbarView-title");
 
-      // 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 &&
-        !this.selectedElement &&
-        (localSearchMode || engine)
-      ) {
-        item.setAttribute("show-action-text", "true");
-      } else {
-        item.removeAttribute("show-action-text");
-      }
-
       // If an engine is selected, update search results to use that engine.
       // Otherwise, restore their original engines.
       if (result.type == UrlbarUtils.RESULT_TYPE.SEARCH) {
@@ -1995,14 +1983,19 @@ class UrlbarView {
         continue;
       }
 
-      // If the result is the heuristic, update its title to reflect the search
-      // string. This means we restyle it to look like a search result. We
+      // If the result is the heuristic and a one-off is selected (i.e.,
+      // localSearchMode || engine), then restyle it to look like a search
+      // result; otherwise, remove such styling. For restyled results, we
       // override the usual result-picking behaviour in UrlbarInput.pickResult.
       if (this.oneOffsRefresh && result.heuristic) {
         title.textContent =
           localSearchMode || engine
             ? this._queryContext.searchString
             : result.title;
+
+        // Set the restyled-search attribute so the action text and title
+        // separator are shown or hidden via CSS as appropriate.
+        item.toggleAttribute("restyled-search", localSearchMode || engine);
       }
 
       // Update result action text.

browser/components/urlbar/tests/browser/browser_oneOffs_heuristicRestyle.js

@@ -86,6 +86,17 @@ async function heuristicIsNotRestyled(expectedType, resultDetails) {
     "The result has the expected non-styled action text."
   );
 
+  Assert.equal(
+    !!BrowserTestUtils.is_visible(resultDetails.element.separator),
+    !!actionText,
+    "The title separator is " + (actionText ? "visible" : "hidden")
+  );
+  Assert.equal(
+    !!BrowserTestUtils.is_visible(resultDetails.element.action),
+    !!actionText,
+    "The action text is " + (actionText ? "visible" : "hidden")
+  );
+
   Assert.equal(
     resultDetails.image,
     data.icon,
@@ -153,6 +164,15 @@ async function heuristicIsRestyled(
     "The restyled result's title should be equal to the search string."
   );
 
+  Assert.ok(
+    BrowserTestUtils.is_visible(resultDetails.element.separator),
+    "The restyled result's title separator should be visible"
+  );
+  Assert.ok(
+    BrowserTestUtils.is_visible(resultDetails.element.action),
+    "The restyled result's action text should be visible"
+  );
+
   if (engine) {
     Assert.equal(
       resultDetails.image,

browser/themes/shared/urlbarView.inc.css

@@ -495,7 +495,8 @@
 }
 
 .urlbarView-row:is([type=remotetab], [sponsored]):not([selected], :hover) > .urlbarView-row-inner > .urlbarView-url,
-.urlbarView-row[type=search]:not([selected], :hover, [show-action-text]) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title-separator,
+.urlbarView-row[type=search]:not([selected], [restyled-search], :hover) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title-separator,
+.urlbarView-row:not([has-action], [has-url], [restyled-search]) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-title-separator,
 .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
@@ -504,7 +505,8 @@
 }
 
 .urlbarView-row:is([type=remotetab], [sponsored]):is([selected], :hover) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-action,
-.urlbarView-row[type=search]:not([selected], :hover, [show-action-text]) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-action,
+.urlbarView-row[type=search]:not([selected], [restyled-search], :hover) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-action,
+.urlbarView-row:not([has-action], [has-url], [restyled-search]) > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-action,
 .urlbarView[actionoverride] .urlbarView-row[type=switchtab] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-action {
   display: none;
 }