Settings: All Sites now shows more user-friendly URLs.

Currently, All Sites shows all origins in their raw format, which looks crowded
and can be confusing to users. Use the host instead in most situations,
displaying non-HTTPS schemes off to the side to easily differentiate from other
hosts that may be the same. Note non-default ports will also be shown as part of
the host.

See a screenshot -
https://bugs.chromium.org/p/chromium/issues/detail?id=835712#c12

Bug: 835712
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Id34ef01e3fe41cd428e5a97bbc4fc207dbe07d05
Reviewed-on: https://chromium-review.googlesource.com/1107579
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571406}

chrome/app/settings_strings.grdp

@@ -2648,6 +2648,9 @@
   <message name="IDS_SETTINGS_SITE_SETTINGS_ALL_SITES_DESCRIPTION" desc="Label for the button linking to the All Sites page.">
     View permissions and data stored across sites
   </message>
+  <message name="IDS_SETTINGS_SITE_SETTINGS_SITE_REPRESENTATION_SEPARATOR" desc="The separator used to split up the display of a URL into host and scheme/protocol, when the scheme is not HTTPS. For example, it will be used in a string that looks like 'google.co.uk — http'.">
+      —
+  </message>
   <message name="IDS_SETTINGS_SITE_SETTINGS_ADS" desc="Label for the ads site settings.">
     Ads
   </message>

chrome/browser/resources/settings/site_settings/site_entry.html

@@ -15,6 +15,17 @@
         display: flex;
         flex-direction: row;
       }
+
+      /* URLs should always be displayed in a LTR embedding - see
+       * https://url.spec.whatwg.org/#url-rendering. */
+      .url-directionality {
+        direction: ltr;
+        unicode-bidi: embed;
+      }
+
+      .site-representation {
+        display: flex;
+      }
     </style>
     <div id="collapseParent">
       <div class="settings-box list-item">
@@ -23,8 +34,14 @@
           <div class="favicon-image"
               style$="[[getSiteGroupIcon_(siteGroup)]]">
           </div>
-          <div class="middle text-elide" id="displayName">
-            [[displayName_]]
+          <div class="site-representation middle text-elide" id="displayName">
+            <span class="url-directionality">[[displayName_]]</span>
+            <span class="secondary" hidden$="[[!scheme_(siteGroup)]]">
+              &nbsp;$i18nPolymer{siteSettingsSiteRepresentationSeparator}&nbsp;
+            </span>
+            <span class="secondary" hidden$="[[!scheme_(siteGroup)]]">
+              [[scheme_(siteGroup)]]
+            </span>
           </div>
           <div hidden$="[[!grouped_(siteGroup)]]">
             <paper-icon-button-light id="expandIcon" class="icon-expand-more">
@@ -57,8 +74,18 @@
               <div class="favicon-image"
                   style$="[[computeSiteIcon(item)]]">
               </div>
-              <div class="middle text-elide">
-                [[item]]
+              <div class="site-representation middle text-elide">
+                <span class="url-directionality">
+                  [[siteRepresentation_(siteGroup, index)]]
+                </span>
+                <span class="secondary"
+                    hidden$="[[!scheme_(siteGroup, index)]]">
+                  &nbsp;$i18nPolymer{siteSettingsSiteRepresentationSeparator}&nbsp;
+                </span>
+                <span class="secondary"
+                    hidden$="[[!scheme_(siteGroup, index)]]">
+                  [[scheme_(siteGroup, index)]]
+                </span>
               </div>
             </div>
           </template>

chrome/browser/resources/settings/site_settings/site_entry.js

@@ -24,9 +24,7 @@ Polymer({
 
     /**
      * The name to display beside the icon. If grouped_() is true, it will be
-     * the eTLD+1 for all the origins, otherwise, it will match the origin more
-     * closely in an appropriate site representation.
-     * @type {!string}
+     * the eTLD+1 for all the origins, otherwise, it will return the host.
      * @private
      */
     displayName_: String,
@@ -43,15 +41,61 @@ Polymer({
     return siteGroup.origins.length != 1;
   },
 
+  /**
+   * Returns a user-friendly name for the origin corresponding to |originIndex|.
+   * If grouped_() is true and |originIndex| is not provided, returns the eTLD+1
+   * for all the origins, otherwise, return the host for that origin.
+   * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
+   * @param {!number=} originIndex Optional index of the origin to get a
+   *     user-friendly name for. If not provided, returns the eTLD+1 name, if
+   *     there is one, otherwise defaults to the first origin.
+   * @return {string} The user-friendly name.
+   * @private
+   */
+  siteRepresentation_: function(siteGroup, originIndex) {
+    if (!siteGroup)
+      return '';
+    if (this.grouped_(siteGroup) && originIndex === undefined) {
+      if (siteGroup.etldPlus1 != '')
+        return siteGroup.etldPlus1;
+      // Fall back onto using the host of the first origin, if no eTLD+1 name
+      // was computed.
+      return this.toUrl(siteGroup.origins[0]).host;
+    }
+    originIndex = originIndex === undefined ? 0 : originIndex;
+    const url = this.toUrl(siteGroup.origins[originIndex]);
+    return url.host;
+  },
+
   /**
    * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
    * @private
    */
   onSiteGroupChanged_: function(siteGroup) {
-    // TODO(https://crbug.com/835712): Present the origin in a user-friendly
-    // site representation when ungrouped.
-    this.displayName_ =
-        this.grouped_(siteGroup) ? siteGroup.etldPlus1 : siteGroup.origins[0];
+    this.displayName_ = this.siteRepresentation_(siteGroup);
+  },
+
+  /**
+   * Returns any non-HTTPS scheme/protocol for the origin corresponding to
+   * |originIndex|. Otherwise, returns a empty string.
+   * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
+   * @param {!number=} originIndex Optional index of the origin to get the
+   *     non-HTTPS scheme for. If not provided, returns an empty string for
+   *     grouped |siteGroup|s but defaults to 0 for non-grouped.
+   * @return {string} The scheme if non-HTTPS, or empty string if HTTPS.
+   * @private
+   */
+  scheme_: function(siteGroup, originIndex) {
+    if (!siteGroup || (this.grouped_(siteGroup) && originIndex === undefined))
+      return '';
+    originIndex = originIndex === undefined ? 0 : originIndex;
+
+    const url = this.toUrl(siteGroup.origins[originIndex]);
+    const scheme = url.protocol.replace(new RegExp(':*$'), '');
+    /** @type{string} */ const HTTPS_SCHEME = 'https';
+    if (scheme == HTTPS_SCHEME)
+      return '';
+    return scheme;
   },
 
   /**

chrome/browser/resources/settings/site_settings/site_settings_behavior.js

@@ -188,7 +188,6 @@ const SiteSettingsBehaviorImpl = {
    */
   getCategoryList: function() {
     if (this.contentTypes_.length == 0) {
-      /** @type {!Array<settings.ContentSettingsTypes>} */
       for (let typeName in settings.ContentSettingsTypes) {
         const contentType = settings.ContentSettingsTypes[typeName];
         // <if expr="not chromeos">

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -2180,6 +2180,8 @@ void AddSiteSettingsStrings(content::WebUIDataSource* html_source,
     {"siteSettingsAllSites", IDS_SETTINGS_SITE_SETTINGS_ALL_SITES},
     {"siteSettingsAllSitesDescription",
      IDS_SETTINGS_SITE_SETTINGS_ALL_SITES_DESCRIPTION},
+    {"siteSettingsSiteRepresentationSeparator",
+     IDS_SETTINGS_SITE_SETTINGS_SITE_REPRESENTATION_SEPARATOR},
     {"siteSettingsAutomaticDownloads",
      IDS_SETTINGS_SITE_SETTINGS_AUTOMATIC_DOWNLOADS},
     {"siteSettingsBackgroundSync", IDS_SETTINGS_SITE_SETTINGS_BACKGROUND_SYNC},