Bug 1391422: Replace java.net.URI with android.net.Uri in getFormattedDomain. r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Tue, 22 Aug 2017 16:12:23 -0700
changeset 650848 e5f98775d8a70810105c620e22e1a9184df6dc8c
parent 646884 7ff4c2f1fe11f6b98686f783294692893b1e1e8b
child 727510 c2574a67736f29e951d4cfe494eaf1957fda6aa6
push id75510
push usermichael.l.comella@gmail.com
push dateTue, 22 Aug 2017 23:18:29 +0000
reviewerssebastian
bugs1391422
milestone57.0a1
Bug 1391422: Replace java.net.URI with android.net.Uri in getFormattedDomain. r=sebastian java.net.URI does not support unicode everywhere, including the host, which would make it impossible to show formatted unicode domains in the top sites. This does not fix the issue however. MozReview-Commit-ID: Dlkkf9CWzaj
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
@@ -1,16 +1,17 @@
 /* -*- 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.menu;
 
 import android.app.Activity;
 import android.content.Context;
+import android.net.Uri;
 import android.os.AsyncTask;
 import android.support.annotation.Nullable;
 import android.support.design.widget.BottomSheetBehavior;
 import android.support.design.widget.BottomSheetDialog;
 import android.support.design.widget.NavigationView;
 import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.MenuItem;
@@ -67,26 +68,26 @@ import java.net.URISyntaxException;
 
         bottomSheetDialog.setContentView(content);
 
         final String pageTitle = item.getTitle();
         final String sheetPageTitle = !TextUtils.isEmpty(pageTitle) ? pageTitle : item.getUrl();
         ((TextView) content.findViewById(R.id.title)).setText(sheetPageTitle);
 
         final TextView pageDomainView = (TextView) content.findViewById(R.id.url);
-        final URI itemURI;
-        try {
-            itemURI = new URI(item.getUrl());
+        final String urlString = item.getUrl();
+        if (urlString == null) {
+            // Invalid URI: not much processing we can do. Like the async task, the page title view sets itself to the
+            // url on error so we leave this field blank.
+            pageDomainView.setText("");
+        } else {
+            final Uri itemURI = Uri.parse(item.getUrl());
             final UpdatePageDomainAsyncTask updateDomainAsyncTask = new UpdatePageDomainAsyncTask(context, pageDomainView,
                     itemURI);
             updateDomainAsyncTask.execute();
-        } catch (final URISyntaxException e) {
-            // Invalid URI: not much processing we can do. Like the async task, the page title view sets itself to the
-            // url on error so we leave this field blank.
-            pageDomainView.setText("");
         }
 
         // Copy layouted parameters from the Highlights / TopSites items to ensure consistency
         final FaviconView faviconView = (FaviconView) content.findViewById(R.id.icon);
         ViewGroup.LayoutParams layoutParams = faviconView.getLayoutParams();
         layoutParams.width = tilesWidth;
         layoutParams.height = tilesHeight;
         faviconView.setLayoutParams(layoutParams);
@@ -134,17 +135,17 @@ import java.net.URISyntaxException;
     public void dismiss() {
         bottomSheetDialog.dismiss();
     }
 
     /** Updates the given TextView's text to the page domain. */
     private static class UpdatePageDomainAsyncTask extends URIUtils.GetFormattedDomainAsyncTask {
         private final WeakReference<TextView> pageDomainViewWeakReference;
 
-        private UpdatePageDomainAsyncTask(final Context context, final TextView pageDomainView, final URI uri) {
+        private UpdatePageDomainAsyncTask(final Context context, final TextView pageDomainView, final Uri uri) {
             super(context, uri, true, 0); // baseDomain.
             this.pageDomainViewWeakReference = new WeakReference<>(pageDomainView);
         }
 
         @Override
         protected void onPostExecute(final String baseDomain) {
             super.onPostExecute(baseDomain);
 
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
@@ -2,16 +2,17 @@
  * 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.stream;
 
 import android.content.Context;
 import android.graphics.Color;
+import android.net.Uri;
 import android.support.annotation.UiThread;
 import android.text.TextUtils;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Telemetry;
@@ -129,38 +130,36 @@ public class HighlightItem extends Strea
             default:
                 pageSourceView.setVisibility(View.INVISIBLE);
                 pageSourceIconView.setImageResource(0);
                 break;
         }
     }
 
     private void updatePageDomain() {
-        final URI highlightURI;
-        try {
-            highlightURI = new URI(highlight.getUrl());
-        } catch (final URISyntaxException e) {
+        final String urlStr = highlight.getUrl();
+        if (urlStr == null) {
             // If the URL is invalid, there's not much extra processing we can do on it.
             pageDomainView.setText(highlight.getUrl());
             return;
         }
 
         final UpdatePageDomainAsyncTask updatePageDomainTask = new UpdatePageDomainAsyncTask(itemView.getContext(),
-                highlightURI, pageDomainView);
+                Uri.parse(urlStr), pageDomainView);
         updatePageDomainTask.execute();
     }
 
     /** Updates the text of the given view to the host second level domain. */
     private static class UpdatePageDomainAsyncTask extends URIUtils.GetFormattedDomainAsyncTask {
         private static final int VIEW_TAG_ID = R.id.page; // same as the view.
 
         private final WeakReference<TextView> pageDomainViewWeakReference;
         private final UUID viewTagAtStart;
 
-        UpdatePageDomainAsyncTask(final Context contextReference, final URI uri, final TextView pageDomainView) {
+        UpdatePageDomainAsyncTask(final Context contextReference, final Uri uri, final TextView pageDomainView) {
             super(contextReference, uri, false, 0); // hostSLD.
             this.pageDomainViewWeakReference = new WeakReference<>(pageDomainView);
 
             // See isTagSameAsStartTag for details.
             viewTagAtStart = UUID.randomUUID();
             pageDomainView.setTag(VIEW_TAG_ID, viewTagAtStart);
         }
 
--- 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
@@ -2,16 +2,17 @@
  * 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.topsites;
 
 import android.content.Context;
 import android.graphics.Color;
 import android.graphics.drawable.Drawable;
+import android.net.Uri;
 import android.support.annotation.UiThread;
 import android.support.v4.widget.TextViewCompat;
 import android.support.v7.widget.RecyclerView;
 import android.text.TextUtils;
 import android.view.View;
 import android.widget.FrameLayout;
 import android.widget.TextView;
 import org.mozilla.gecko.R;
@@ -105,30 +106,28 @@ 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);
 
-        final URI topSiteURI;
-        try {
-            topSiteURI = new URI(topSite.getUrl());
-        } catch (final URISyntaxException e) {
+        final String urlString = topSite.getUrl();
+        if (urlString == null) {
             // If this is not a valid URI, there is not much processing we can do on it.
             // Also, see comment below regarding setCenteredText.
             setTopSiteTitle(title, topSite.getUrl());
             return;
         }
 
         // 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);
+                Uri.parse(urlString), title);
         titleAsyncTask.execute();
     }
 
     private static void setTopSiteTitle(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());
     }
@@ -140,17 +139,17 @@ import java.util.concurrent.Future;
 
     /** Updates the text of the given view to the page domain. */
     private static class UpdateCardTitleAsyncTask extends URIUtils.GetFormattedDomainAsyncTask {
         private static final int VIEW_TAG_ID = R.id.title; // same as the view.
 
         private final WeakReference<TextView> titleViewWeakReference;
         private final UUID viewTagAtStart;
 
-        UpdateCardTitleAsyncTask(final Context contextReference, final URI uri, final TextView titleView) {
+        UpdateCardTitleAsyncTask(final Context contextReference, final Uri uri, final TextView titleView) {
             super(contextReference, uri, false, 1); // subdomain.domain.
             this.titleViewWeakReference = new WeakReference<>(titleView);
 
             // See isTagSameAsStartTag for details.
             viewTagAtStart = UUID.randomUUID();
             titleView.setTag(VIEW_TAG_ID, viewTagAtStart);
         }
 
--- a/mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
@@ -1,15 +1,16 @@
 /* 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.util;
 
 import android.content.Context;
+import android.net.Uri;
 import android.os.AsyncTask;
 import android.support.annotation.IntRange;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.WorkerThread;
 import android.text.TextUtils;
 import android.util.Log;
 import ch.boye.httpclientandroidlib.conn.util.InetAddressUtils;
@@ -66,17 +67,17 @@ public class URIUtils {
      * @param uri the URI whose host we should format.
      * @param shouldIncludePublicSuffix true if the public suffix should be included, false otherwise.
      * @param subdomainCount The number of subdomains to include.
      *
      * @return the formatted domain, or the empty String if the host cannot be found.
      */
     @NonNull
     @WorkerThread // calls PublicSuffix methods.
-    public static String getFormattedDomain(@NonNull final Context context, @NonNull final URI uri,
+    public static String getFormattedDomain(@NonNull final Context context, @NonNull final Uri uri,
             final boolean shouldIncludePublicSuffix, @IntRange(from = 0) final int subdomainCount) {
         if (context == null) { throw new NullPointerException("Expected non-null Context argument"); }
         if (uri == null) { throw new NullPointerException("Expected non-null uri argument"); }
         if (subdomainCount < 0) { throw new IllegalArgumentException("Expected subdomainCount >= 0."); }
 
         final String host = uri.getHost();
         if (TextUtils.isEmpty(host)) {
             return ""; // There's no host so there's no domain to retrieve.
@@ -115,34 +116,34 @@ public class URIUtils {
             }
         }
 
         // There are fewer subdomains than the total we'll accept so return them all!
         return host;
     }
 
     // impl via FFiOS: https://github.com/mozilla-mobile/firefox-ios/blob/deb9736c905cdf06822ecc4a20152df7b342925d/Shared/Extensions/NSURLExtensions.swift#L292
-    private static boolean isIPv6(final URI uri) {
+    private static boolean isIPv6(final Uri uri) {
         final String host = uri.getHost();
         return !TextUtils.isEmpty(host) && host.contains(":");
     }
 
     /**
      * An async task that will take a URI formatted as a String and will retrieve
-     * {@link #getFormattedDomain(Context, URI, boolean, int)}. To use this, extend the class and override
+     * {@link #getFormattedDomain(Context, Uri, boolean, int)}. To use this, extend the class and override
      * {@link #onPostExecute(Object)}, where the formatted domain, or the empty String if the host cannot determined,
      * will be returned.
      */
     public static abstract class GetFormattedDomainAsyncTask extends AsyncTask<Void, Void, String> {
         protected final WeakReference<Context> contextWeakReference;
-        protected final URI uri;
+        protected final Uri uri;
         protected final boolean shouldIncludePublicSuffix;
         protected final int subdomainCount;
 
-        public GetFormattedDomainAsyncTask(final Context context, final URI uri, final boolean shouldIncludePublicSuffix,
+        public GetFormattedDomainAsyncTask(final Context context, final Uri uri, final boolean shouldIncludePublicSuffix,
                 final int subdomainCount) {
             this.contextWeakReference = new WeakReference<>(context);
             this.uri = uri;
             this.shouldIncludePublicSuffix = shouldIncludePublicSuffix;
             this.subdomainCount = subdomainCount;
         }
 
         @Override
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
@@ -1,14 +1,15 @@
 /* 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.util;
 
+import android.net.Uri;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.robolectric.RuntimeEnvironment;
 
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -143,32 +144,32 @@ public class TestURIUtils {
     }
 
     @Test
     public void testGetFormattedDomainIPv6() throws Exception {
         assertGetFormattedDomain("http://[3ffe:1900:4545:3:200:f8ff:fe21:67cf]", false, 0, "[3ffe:1900:4545:3:200:f8ff:fe21:67cf]");
         assertGetFormattedDomain("http://[3ffe:1900:4545:3:200:f8ff:fe21:67cf]", true, 0, "[3ffe:1900:4545:3:200:f8ff:fe21:67cf]");
     }
 
+    @Test
+    public void testGetFormattedDomainUnicode() throws Exception {
+        assertGetFormattedDomain("http://faß.de/", false, 0, "faß");
+        assertGetFormattedDomain("http://faß.de/", true, 0, "faß.de");
+    }
+
     @Test(expected = NullPointerException.class)
     public void testGetFormattedDomainNullContextThrows() throws Exception {
-        URIUtils.getFormattedDomain(null, new URI("http://google.com"), false, 0);
+        URIUtils.getFormattedDomain(null, Uri.parse("http://google.com"), false, 0);
     }
 
     @Test(expected = NullPointerException.class)
     public void testGetFormattedDomainNullURIThrows() throws Exception {
         URIUtils.getFormattedDomain(RuntimeEnvironment.application, null, false, 0);
     }
 
     private void assertGetFormattedDomain(final String uriString, final boolean includePublicSuffix,
             final int subdomainCount, final String expected) {
-        final URI uri;
-        try {
-            uri = new URI(uriString);
-        } catch (final URISyntaxException e) {
-            throw new IllegalArgumentException("Invalid URI passed into test: " + uriString);
-        }
-
+        final Uri uri = Uri.parse(uriString);
         Assert.assertEquals("for input:" + uriString + "||",
                 expected,
                URIUtils.getFormattedDomain(RuntimeEnvironment.application, uri, includePublicSuffix, subdomainCount));
     }
 }
\ No newline at end of file