Bug 1567301 - UnifiedComplete: Change the title-tags separator to a non-printable character. r=mak

This just changes the `TITLE_TAGS_SEPARATOR` to the non-printable character `\x1F`, the unit separator, which seems appropriate.

At first I thought we could use the result label to store tags since we're not using the label at all right now. Hacky, but better than storing them in the title. But (1) `nsAutoCompleteSimpleResult::GetLabelAt` falls back to the value if the label is empty, and (2) `nsAutoCompleteController::GetLabelAt` actually returns the same thing as `GetValueAt`, i.e., the value, not the label. It's doable, but we'd need set the label to some special value for every result that doesn't have tags so that the label doesn't fall back to the value so we can tell which results don't have tags, and we'd need to make sure to always directly ask results for labels instead of going through the controller.  head_autocomplete.js goes through the controller, and I didn't check what else does too.

So then I thought we could store tags in the style with a special substring like "tags=tag1,tag2,tag3". Again it's doable, but:

The simplest fix is to just change the separator to an unprintable character. That should work, right? We can do better whenever we finally rewrite/refactor UnifiedComplete for quantumbar.

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

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

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -36,9 +36,6 @@ XPCOMUtils.defineLazyGetter(this, "logger", () =>
   Log.repository.getLogger("Urlbar.Provider.UnifiedComplete")
 );
 
-// See UnifiedComplete.
-const TITLE_TAGS_SEPARATOR = " \u2013 ";
-
 /**
  * Class used to create the provider.
  */
@@ -210,7 +207,6 @@ function convertResultToMatches(context, result, urls) {
       continue;
     }
     urls.add(url);
-    // Not used yet: result.getLabelAt(i)
     let style = result.getStyleAt(i);
     let match = makeUrlbarResult(context.tokens, {
       url,
@@ -388,7 +384,7 @@ function makeUrlbarResult(tokens, info) {
     source = UrlbarUtils.RESULT_SOURCE.BOOKMARKS;
     if (info.style.includes("tag")) {
       // Split title and tags.
-      [comment, tags] = info.comment.split(TITLE_TAGS_SEPARATOR);
+      [comment, tags] = info.comment.split(UrlbarUtils.TITLE_TAGS_SEPARATOR);
       // Tags are separated by a comma and in a random order.
       // We should also just include tags that match the searchString.
       tags = tags

browser/components/urlbar/UrlbarUtils.jsm

@@ -135,6 +135,14 @@ var UrlbarUtils = {
     SUGGESTED: 2,
   },
 
+  // UnifiedComplete's autocomplete results store their titles and tags together
+  // in their comments.  This separator is used to separate them.  When we
+  // rewrite UnifiedComplete for quantumbar, we should stop using this old hack
+  // and store titles and tags separately.  It's important that this be a
+  // character that no title would ever have.  We use \x1F, the non-printable
+  // unit separator.
+  TITLE_TAGS_SEPARATOR: "\x1F",
+
   /**
    * Adds a url to history as long as it isn't in a private browsing window,
    * and it is valid.

toolkit/components/places/UnifiedComplete.jsm

@@ -18,11 +18,6 @@ const QUERYTYPE_AUTOFILL_ORIGIN = 1;
 const QUERYTYPE_AUTOFILL_URL = 2;
 const QUERYTYPE_ADAPTIVE = 3;
 
-// This separator is used as an RTL-friendly way to split the title and tags.
-// It can also be used by an nsIAutoCompleteResult consumer to re-split the
-// "comment" back into the title and the tag.
-const TITLE_TAGS_SEPARATOR = " \u2013 ";
-
 // Telemetry probes.
 const TELEMETRY_1ST_RESULT = "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS";
 const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
@@ -2425,7 +2420,7 @@ Search.prototype = {
 
     // If we have tags and should show them, we need to add them to the title.
     if (showTags) {
-      title += TITLE_TAGS_SEPARATOR + tags;
+      title += UrlbarUtils.TITLE_TAGS_SEPARATOR + tags;
     }
 
     // We have to determine the right style to display.  Tags show the tag icon,

toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js

@@ -58,10 +58,9 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.jsm",
   UrlbarTokenizer: "resource:///modules/UrlbarTokenizer.jsm",
+  UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
 });
 
-const TITLE_SEARCH_ENGINE_SEPARATOR = " \u00B7\u2013\u00B7 ";
-
 const { AddonTestUtils } = ChromeUtils.import(
   "resource://testing-common/AddonTestUtils.jsm"
 );
@@ -170,7 +169,7 @@ async function _check_autocomplete_matches(match, result) {
   let title = match.comment || match.title;
 
   if (tags) {
-    title += " \u2013 " + tags.sort().join(", ");
+    title += UrlbarUtils.TITLE_TAGS_SEPARATOR + tags.sort().join(", ");
   }
   if (style) {
     style = style.sort();