From 034b23ff4165bb9f84e4d6a4fbb52be75cc1a7b1 Mon Sep 17 00:00:00 2001
From: Michael Comella <michael.l.comella@gmail.com>
Date: Wed, 23 Sep 2015 14:48:28 -0700
Subject: [PATCH] Bug 1173147 - Explicitly show Android chooser when there is
 more than one intent URI match in pb. r=sebastian

After this changeset series, the expected flow for web links is:
  * If not pb, open the Intent URI
  * If pb and no matching applications, open about:neterror
  * If pb and one matching application, show this dialog
  * If pb and > 1 matching application, show the Android system chooser

When the user explicitly chooses to share (and thus should infer they're
exiting Private Browsing), we don't show the dialog.

Custom URIs sort of work: I tested `mailto` and it worked as expected but `tel`
does not work as expected (i.e. it doesn't show the dialog). Perhaps there's an
explicit "Open dialer" code path. To figure this out, I tested this patch
against my Intent URI test page [1].

Decisions around explicitly showing the Android chooser:
When there are multiple application matches to an Intent URI, we
want to show the Android Intent Chooser. However, we have no way
of distinguishing regular tabs from private tabs to the chooser.
Thus, if a user chooses "Always" in regular browsing mode, the
chooser will not be shown and the URL will be opened. Therefore we
explicitly show the chooser (which notably does not have an "Always"
option).

[1]: https://people.mozilla.org/~mcomella/test/uri.html

--HG--
extra : commitid : 1cCVQe5jmNx
extra : rebase_source : 146766c2ac5cf8814288377453253debc2ff3f8a
---
 mobile/android/base/locales/en-US/android_strings.dtd |  8 ++++++++
 mobile/android/base/strings.xml.in                    |  1 +
 ...rnalIntentDuringPrivateBrowsingPromptFragment.java | 11 ++++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/mobile/android/base/locales/en-US/android_strings.dtd b/mobile/android/base/locales/en-US/android_strings.dtd
index a448e4ef5786..8058cae1ddf4 100644
--- a/mobile/android/base/locales/en-US/android_strings.dtd
+++ b/mobile/android/base/locales/en-US/android_strings.dtd
@@ -710,6 +710,14 @@ just addresses the organization to follow, e.g. "This site is run by " -->
      clicks a link that will open an external Android application. "&formatS;"
      will be replaced with the name of the application that will be opened. -->
 <!ENTITY intent_uri_private_browsing_prompt "This link will open in &formatS;. Are you sure you want to exit Private Browsing?">
+<!-- LOCALIZATION NOTE (intent_uri_private_browsing_multiple_match_title): This
+     string will appear as the title of an alert when a user, who is currently
+     in private browsing, clicks a link that will open an external Android
+     application and more than one application is available to open that link.
+     We don't have control over the style of this dialog and it looks
+     unpolished when this string is longer than one line so ideally keep it
+     short! -->
+<!ENTITY intent_uri_private_browsing_multiple_match_title "Exit Private Browsing?">
 
 <!-- DevTools Authentication -->
 <!-- LOCALIZATION NOTE (devtools_auth_scan_header): This header text appears
diff --git a/mobile/android/base/strings.xml.in b/mobile/android/base/strings.xml.in
index 5231962ed067..a6f78af583f8 100644
--- a/mobile/android/base/strings.xml.in
+++ b/mobile/android/base/strings.xml.in
@@ -596,6 +596,7 @@
 
   <string name="intent_uri_cannot_open">&intent_uri_cannot_open;</string>
   <string name="intent_uri_private_browsing_prompt">&intent_uri_private_browsing_prompt;</string>
+  <string name="intent_uri_private_browsing_multiple_match_title">&intent_uri_private_browsing_multiple_match_title;</string>
 
   <string name="devtools_auth_scan_header">&devtools_auth_scan_header;</string>
 </resources>
diff --git a/mobile/android/base/widget/ExternalIntentDuringPrivateBrowsingPromptFragment.java b/mobile/android/base/widget/ExternalIntentDuringPrivateBrowsingPromptFragment.java
index 067077c2b388..0b075eb41125 100644
--- a/mobile/android/base/widget/ExternalIntentDuringPrivateBrowsingPromptFragment.java
+++ b/mobile/android/base/widget/ExternalIntentDuringPrivateBrowsingPromptFragment.java
@@ -80,9 +80,14 @@ public class ExternalIntentDuringPrivateBrowsingPromptFragment extends DialogFra
             // We don't know the results of the user interaction with the fragment so just return true.
             return true;
         } else if (matchingActivities.size() > 1) {
-            // Android chooser dialog will be shown, which should make the users
-            // aware they're entering a new application from private browsing.
-            return ActivityHandlerHelper.startIntentAndCatch(LOGTAG, context, intent);
+            // We want to show the Android Intent Chooser. However, we have no way of distinguishing regular tabs from
+            // private tabs to the chooser. Thus, if a user chooses "Always" in regular browsing mode, the chooser will
+            // not be shown and the URL will be opened. Therefore we explicitly show the chooser (which notably does not
+            // have an "Always" option).
+            final String androidChooserTitle =
+                    context.getResources().getString(R.string.intent_uri_private_browsing_multiple_match_title);
+            final Intent chooserIntent = Intent.createChooser(intent, androidChooserTitle);
+            return ActivityHandlerHelper.startIntentAndCatch(LOGTAG, context, chooserIntent);
         } else {
             // Normally, we show about:neterror when an Intent does not resolve
             // but we don't have the references here to do that so log instead.