Bug 1391413: Always show page titles for distributions. r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 07 Sep 2017 15:30:00 -0700
changeset 661133 90c3a58b05c1a6fcaff56e7524b3d0f6c851e9cd
parent 661132 432c1cdab9bc37473cb9da72b066153a83c45139
child 661134 c8767e78adc54e00fef97a1a968c3a187c3d8e18
push id78639
push usermichael.l.comella@gmail.com
push dateFri, 08 Sep 2017 00:35:03 +0000
reviewerssebastian
bugs1391413
milestone57.0a1
Bug 1391413: Always show page titles for distributions. r=sebastian This is the desired behavior but as per comment 8, it breaks a few existing distributions' titles so we'll display two lines for those distributions in the upcoming changesets. MozReview-Commit-ID: CKFbHXbs3HT
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
@@ -15,16 +15,17 @@ import android.view.View;
 import android.widget.FrameLayout;
 import android.widget.TextView;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.activitystream.ActivityStreamTelemetry;
 import org.mozilla.gecko.activitystream.homepanel.menu.ActivityStreamContextMenu;
 import org.mozilla.gecko.activitystream.homepanel.model.TopSite;
+import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.home.HomePager;
 import org.mozilla.gecko.icons.IconCallback;
 import org.mozilla.gecko.icons.IconResponse;
 import org.mozilla.gecko.icons.Icons;
 import org.mozilla.gecko.util.DrawableUtil;
 import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.util.URIUtils;
 import org.mozilla.gecko.util.ViewUtil;
@@ -117,23 +118,29 @@ import java.util.concurrent.Future;
         URI topSiteURI = null; // not final so we can use in the Exception case.
         boolean wasException = false;
         try {
             topSiteURI = new URI(topSite.getUrl());
         } catch (final URISyntaxException e) {
             wasException = true;
         }
 
-        // At a high level, the logic is: if the path empty, use "subdomain.domain", otherwise use the
-        // page title. From a UX perspective, people refer to domains by their name ("it's on wikipedia")
-        // and it's a clean look. However, if a url has a path, it will not fit on the screen with the domain
-        // so we need an alternative: the page title is an easy win (though not always perfect, e.g. when SEO
-        // keywords are added). If we ever want better titles, we could create a heuristic to pull the title
-        // from parts of the URL, page title, etc.
-        if (wasException || !URIUtils.isPathEmpty(topSiteURI)) {
+        final boolean isSiteSuggestedFromDistribution = BrowserDB.from(itemView.getContext()).getSuggestedSites()
+                .containsSiteAndSiteIsFromDistribution(topSite.getUrl());
+
+        // At a high level, the logic is: if the path non-empty or the site is suggested by a distribution, use the page
+        // title, otherwise use "subdomain.domain". From a UX perspective, people refer to domains by their name ("it's
+        // on wikipedia") and it's a clean look. However, if a url has a path, it will not fit on the screen with the
+        // domain so we need an alternative: the page title is an easy win (though not always perfect, e.g. when SEO
+        // keywords are added). We use page titles with distributions because that's what those distributions expect to
+        // be shown. If we ever want better top site titles, we could create a heuristic to pull the title from parts
+        // of the URL, page title, etc.
+        if (wasException ||
+                isSiteSuggestedFromDistribution ||
+                !URIUtils.isPathEmpty(topSiteURI)) {
             // See comment below regarding setCenteredText.
             final String pageTitle = topSite.getTitle();
             final String updateText = !TextUtils.isEmpty(pageTitle) ? pageTitle : topSite.getUrl();
             setTopSiteTitleHelper(title, updateText);
 
         } else {
             // Our AsyncTask calls setCenteredText(), which needs to have all drawable's in place to correctly
             // layout the text, so we need to wait with requesting the title until we've set our pin icon.