Bug 1393579: Show subdomain.domain for pages without titles in top sites. r=liuche
MozReview-Commit-ID: 9SugVwZDbD7
--- 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
@@ -111,53 +111,57 @@ import java.util.concurrent.Future;
}
TextViewCompat.setCompoundDrawablesRelativeWithIntrinsicBounds(title, pinDrawable, null, null, null);
setTopSiteTitle(topSite);
}
private void setTopSiteTitle(final TopSite topSite) {
URI topSiteURI = null; // not final so we can use in the Exception case.
- boolean wasException = false;
+ boolean isInvalidURI = false;
try {
topSiteURI = new URI(topSite.getUrl());
} catch (final URISyntaxException e) {
- wasException = true;
+ isInvalidURI = true;
}
final boolean isSiteSuggestedFromDistribution = BrowserDB.from(itemView.getContext()).getSuggestedSites()
.containsSiteAndSiteIsFromDistribution(topSite.getUrl());
// Some already installed distributions are unlikely to be updated (OTA, system) and their suggested
// site titles were written for the old top sites, where we had more room to display titles: we want
// to provide them with more lines. However, it's complex to distinguish a distribution intended for
// the old top sites and the new one so for code simplicity, we allow all distributions more lines for titles.
title.setMaxLines(isSiteSuggestedFromDistribution ? 2 : 1);
- // 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();
+ // We use page titles with distributions because that's what the creators of those distributions expect to
+ // be shown. Also, we need a valid URI for our preferred case so we stop here if we don't have one.
+ final String pageTitle = topSite.getTitle();
+ if (isInvalidURI || isSiteSuggestedFromDistribution) {
final String updateText = !TextUtils.isEmpty(pageTitle) ? pageTitle : topSite.getUrl();
- setTopSiteTitleHelper(title, updateText);
+ setTopSiteTitleHelper(title, updateText); // See comment below regarding setCenteredText.
- } else {
+ // This is our preferred case: we display "subdomain.domain". People refer to sites by their domain ("it's
+ // on wikipedia!") and it's a clean look so we display the domain if we can.
+ //
+ // If the path is non-empty, we'd normally go to the case below (see that comment). However, if there's no
+ // title, we'd prefer to fallback on "subdomain.domain" rather than a url, which is really ugly.
+ } else if (URIUtils.isPathEmpty(topSiteURI) ||
+ (!URIUtils.isPathEmpty(topSiteURI) && TextUtils.isEmpty(pageTitle))) {
// 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.
final UpdateCardTitleAsyncTask titleAsyncTask = new UpdateCardTitleAsyncTask(itemView.getContext(),
topSiteURI, title);
titleAsyncTask.execute();
+
+ // We have a site with a path that has a non-empty title. It'd be impossible to distinguish multiple sites
+ // with "subdomain.domain" so if there's a path, we have to use something else: "domain/path" would overflow
+ // before it's useful so we use the page title.
+ } else {
+ setTopSiteTitleHelper(title, pageTitle); // See comment above regarding setCenteredText.
}
}
private static void setTopSiteTitleHelper(final TextView textView, final String title) {
// We use consistent padding all around the title, and the top padding is never modified,
// so we can pass that in as the default padding:
ViewUtil.setCenteredText(textView, title, textView.getPaddingTop());
}