Bug 1382036: Use provider in top sites tile. r=liuche draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 02 Aug 2017 17:13:06 -0700
changeset 642205 6fc3d7c6b2141be971c1498f7c0045dc38b3f20e
parent 642204 65507616792c990b1230888612dd7ffc13ed32b4
child 724938 66f8783c52c8edf8572dfa06e22844ec884bb921
push id72684
push usermichael.l.comella@gmail.com
push dateMon, 07 Aug 2017 22:40:21 +0000
reviewersliuche
bugs1382036, 1386902
milestone57.0a1
Bug 1382036: Use provider in top sites tile. r=liuche This provider is not as effective as iOS' algorithm: I filed bug 1386902. MozReview-Commit-ID: IK1vPWEcAYz
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
@@ -8,16 +8,17 @@ package org.mozilla.gecko.activitystream
 import android.database.Cursor;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import android.text.format.DateUtils;
 import org.mozilla.gecko.activitystream.Utils;
 import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
 import org.mozilla.gecko.activitystream.ranking.HighlightsRanking;
+import org.mozilla.gecko.db.BrowserContract;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public class Highlight implements Item {
 
     /**
      * A pattern matching a json object containing the key "image_url" and extracting the value. afaik, these urls
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
@@ -1,24 +1,30 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.homepanel.model;
 
+import android.database.Cursor;
 import android.support.annotation.Nullable;
 import android.text.TextUtils;
 import android.util.Log;
 import org.json.JSONException;
 import org.json.JSONObject;
 
 public class Metadata {
     private static final String LOGTAG = "GeckoMetadata";
 
+    public static Metadata fromCursor(final Cursor cursor, final String columnName) {
+        return new Metadata(
+                cursor.getString(cursor.getColumnIndexOrThrow(columnName)));
+    }
+
     private String provider;
     private String imageUrl;
     private int descriptionLength;
 
     /* package-private */ Metadata(String json) {
         if (TextUtils.isEmpty(json)) {
             // Just use default values. It's better to have an empty Metadata object instead of
             // juggling with null values.
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
@@ -12,42 +12,45 @@ import org.mozilla.gecko.db.BrowserContr
 
 public class TopSite implements Item {
     private final long id;
     private final String url;
     private final String title;
     private @Nullable Boolean isBookmarked;
     private final @Nullable boolean isPinned;
     private @BrowserContract.TopSites.TopSiteType final int type;
+    private final Metadata metadata;
 
     public static TopSite fromCursor(Cursor cursor) {
         // The Combined View only contains pages that have been visited at least once, i.e. any
         // page in the TopSites query will contain a HISTORY_ID. _ID however will be 0 for all rows.
         final long id = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID));
         final String url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
         final String title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.TITLE));
         final int type = cursor.getInt(cursor.getColumnIndexOrThrow(BrowserContract.TopSites.TYPE));
+        final Metadata metadata = Metadata.fromCursor(cursor, BrowserContract.TopSites.PAGE_METADATA_JSON);
 
         // We can't figure out bookmark state of a pin or suggested site, so we leave it as unknown to be queried later.
         Boolean isBookmarked = null;
         if (type != BrowserContract.TopSites.TYPE_PINNED &&
                 type != BrowserContract.TopSites.TYPE_SUGGESTED) {
             isBookmarked = !cursor.isNull(cursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID));
         }
 
-        return new TopSite(id, url, title, isBookmarked, type);
+        return new TopSite(id, url, title, isBookmarked, type, metadata);
     }
 
-    private TopSite(long id, String url, String title, @Nullable Boolean isBookmarked, int type) {
+    private TopSite(long id, String url, String title, @Nullable Boolean isBookmarked, int type, final Metadata metadata) {
         this.id = id;
         this.url = url;
         this.title = title;
         this.isBookmarked = isBookmarked;
         this.isPinned = type == BrowserContract.TopSites.TYPE_PINNED;
         this.type = type;
+        this.metadata = metadata;
     }
 
     public long getId() {
         return id;
     }
 
     public String getUrl() {
         return url;
@@ -66,16 +69,20 @@ public class TopSite implements Item {
     public int getType() {
         return type;
     }
 
     public Boolean isPinned() {
         return isPinned;
     }
 
+    public Metadata getMetadata() {
+        return metadata;
+    }
+
     @Override
     public void updateBookmarked(boolean bookmarked) {
         this.isBookmarked = bookmarked;
     }
 
     @Override
     public void updatePinned(boolean pinned) {
         throw new UnsupportedOperationException(
--- 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
@@ -102,30 +102,26 @@ import java.util.concurrent.Future;
         final Drawable pinDrawable;
         if (topSite.isPinned()) {
             pinDrawable = DrawableUtil.tintDrawable(itemView.getContext(), R.drawable.as_pin, Color.WHITE);
         } else {
             pinDrawable = null;
         }
         TextViewCompat.setCompoundDrawablesRelativeWithIntrinsicBounds(title, pinDrawable, null, null, null);
 
-        // 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.
-        //
-        // todo (bug 1382036): iOS' algorithm actually looks like:
-        // if let provider = site.metadata?.providerName {
-        //     titleLabel.text = provider.lowercased()
-        // } else {
-        //     titleLabel.text = site.tileURL.hostSLD
-        // }
-        //
-        // But it's non-trivial to get the provider name with the top site so it's a follow-up.
-        final UpdateCardTitleAsyncTask titleAsyncTask = new UpdateCardTitleAsyncTask(itemView.getContext(),
-                topSite.getUrl(), title);
-        titleAsyncTask.execute();
+        final String provider = topSite.getMetadata().getProvider();
+        if (!TextUtils.isEmpty(provider)) {
+            title.setText(provider.toLowerCase());
+        } 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.
+            final UpdateCardTitleAsyncTask titleAsyncTask = new UpdateCardTitleAsyncTask(itemView.getContext(),
+                    topSite.getUrl(), title);
+            titleAsyncTask.execute();
+        }
     }
 
     @Override
     public void onIconResponse(IconResponse response) {
         faviconView.updateImage(response);
     }
 
     /** Updates the text of the given view to the page domain. */