From c02276b5d16b3698954ba121c2d09cc05b58910a Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Wed, 2 Jun 2021 10:56:32 +0200 Subject: [PATCH 01/26] Add handling of app links --- .../app/browser/BrowserTabViewModelTest.kt | 25 ++-- .../app/browser/BrowserWebViewClientTest.kt | 23 ++++ .../app/browser/SpecialUrlDetectorImplTest.kt | 113 +++++++++++++++++- .../app/settings/SettingsViewModelTest.kt | 12 ++ .../app/browser/BrowserTabFragment.kt | 66 +++++++++- .../app/browser/BrowserTabViewModel.kt | 18 ++- .../app/browser/BrowserWebViewClient.kt | 24 +++- .../app/browser/SpecialUrlDetector.kt | 65 +++++++++- .../app/browser/WebViewClientListener.kt | 3 +- .../app/browser/di/BrowserModule.kt | 2 +- .../app/settings/SettingsActivity.kt | 14 +++ .../app/settings/SettingsViewModel.kt | 12 +- .../app/settings/db/SettingsDataStore.kt | 6 + .../res/layout/content_settings_privacy.xml | 10 ++ app/src/main/res/values/strings.xml | 6 + 15 files changed, 362 insertions(+), 37 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 69f29f556cc6..2c07f423cccc 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -16,6 +16,7 @@ package com.duckduckgo.app.browser +import android.content.Context import android.graphics.Bitmap import android.net.Uri import android.view.MenuItem @@ -264,6 +265,9 @@ class BrowserTabViewModelTest { @Mock private lateinit var mockFavoritesRepository: FavoritesRepository + @Mock + private lateinit var mockContext: Context + private val lazyFaviconManager = Lazy { mockFaviconManager } private lateinit var mockAutoCompleteApi: AutoCompleteApi @@ -352,7 +356,7 @@ class BrowserTabViewModelTest { bookmarksDao = mockBookmarksDao, longPressHandler = mockLongPressHandler, webViewSessionStorage = webViewSessionStorage, - specialUrlDetector = SpecialUrlDetectorImpl(), + specialUrlDetector = SpecialUrlDetectorImpl(mockContext, mockSettingsStore), faviconManager = mockFaviconManager, addToHomeCapabilityDetector = mockAddToHomeCapabilityDetector, ctaViewModel = ctaViewModel, @@ -3011,24 +3015,24 @@ class BrowserTabViewModelTest { @Test fun whenExternalAppLinkClickedIfGpcIsEnabledThenAddHeaderToUrl() { whenever(mockSettingsStore.globalPrivacyControlEnabled).thenReturn(true) - val intentType = SpecialUrlDetector.UrlType.IntentType("query", mock(), null) + val intentType = SpecialUrlDetector.UrlType.NonHttpAppLink("query", mock(), null) - testee.externalAppLinkClicked(intentType) + testee.nonHttpAppLinkClicked(intentType) verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) - val command = commandCaptor.lastValue as Command.HandleExternalAppLink + val command = commandCaptor.lastValue as Command.HandleNonHttpAppLink assertEquals(GPC_HEADER_VALUE, command.headers[GPC_HEADER]) } @Test fun whenExternalAppLinkClickedIfGpcIsDisabledThenDoNotAddHeaderToUrl() { whenever(mockSettingsStore.globalPrivacyControlEnabled).thenReturn(false) - val intentType = SpecialUrlDetector.UrlType.IntentType("query", mock(), null) + val intentType = SpecialUrlDetector.UrlType.NonHttpAppLink("query", mock(), null) - testee.externalAppLinkClicked(intentType) + testee.nonHttpAppLinkClicked(intentType) verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) - val command = commandCaptor.lastValue as Command.HandleExternalAppLink + val command = commandCaptor.lastValue as Command.HandleNonHttpAppLink assertTrue(command.headers.isEmpty()) } @@ -3197,6 +3201,13 @@ class BrowserTabViewModelTest { assertCommandNotIssued() } + @Test + fun whenAppLinkClickedThenAppLinkCommandSent() { + testee.appLinkClicked(SpecialUrlDetector.UrlType.AppLink()) + + assertCommandIssued() + } + private suspend fun givenFireButtonPulsing() { whenever(mockUserStageStore.getUserAppStage()).thenReturn(AppStage.DAX_ONBOARDING) dismissedCtaDaoChannel.send(listOf(DismissedCta(CtaId.DAX_DIALOG_TRACKERS_FOUND))) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 2900cbbe36b1..8056609f37ba 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -38,6 +38,8 @@ import com.duckduckgo.app.globalprivacycontrol.GlobalPrivacyControl import com.duckduckgo.app.runBlocking import com.duckduckgo.app.statistics.store.OfflinePixelCountDataStore import com.nhaarman.mockitokotlin2.* +import junit.framework.TestCase.assertFalse +import junit.framework.TestCase.assertTrue import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineScope import org.junit.Before @@ -68,6 +70,8 @@ class BrowserWebViewClientTest { private val webViewHttpAuthStore: WebViewHttpAuthStore = mock() private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val emailInjector: EmailInjector = mock() + private val webResourceRequest: WebResourceRequest = mock() + private val webViewClientListener: WebViewClientListener = mock() @UiThreadTest @Before @@ -91,6 +95,8 @@ class BrowserWebViewClientTest { emailInjector ) testee.webViewClientListener = listener + whenever(webResourceRequest.isForMainFrame).thenReturn(false) + whenever(webResourceRequest.url).thenReturn(Uri.EMPTY) } @UiThreadTest @@ -261,6 +267,23 @@ class BrowserWebViewClientTest { verify(uncaughtExceptionRepository).recordUncaughtException(exception, UncaughtExceptionSource.SHOULD_OVERRIDE_REQUEST) } + @Test + fun whenAppLinkDetectedAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { + whenever(webResourceRequest.isRedirect).thenReturn(true) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + } + + @Test + fun whenAppLinkDetectedThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + whenever(webResourceRequest.isRedirect).thenReturn(false) + val urlType = SpecialUrlDetector.UrlType.AppLink() + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) + testee.webViewClientListener = webViewClientListener + assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(webViewClientListener).appLinkClicked(urlType) + } + private class TestWebView(context: Context) : WebView(context) companion object { diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index 82e60b6c29f5..8a96f4f1b4f8 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -16,21 +16,48 @@ package com.duckduckgo.app.browser +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.content.pm.ActivityInfo +import android.content.pm.PackageManager +import android.content.pm.ResolveInfo import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.* import com.duckduckgo.app.browser.SpecialUrlDetectorImpl.Companion.EMAIL_MAX_LENGTH import com.duckduckgo.app.browser.SpecialUrlDetectorImpl.Companion.PHONE_MAX_LENGTH import com.duckduckgo.app.browser.SpecialUrlDetectorImpl.Companion.SMS_MAX_LENGTH +import com.duckduckgo.app.settings.db.SettingsDataStore +import com.nhaarman.mockitokotlin2.* +import junit.framework.TestCase.assertNull +import junit.framework.TestCase.assertTrue import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import java.net.URISyntaxException class SpecialUrlDetectorImplTest { lateinit var testee: SpecialUrlDetector + @Mock + lateinit var mockContext: Context + + @Mock + lateinit var mockPackageManager: PackageManager + + @Mock + lateinit var mockSettingsDataStore: SettingsDataStore + @Before fun setup() { - testee = SpecialUrlDetectorImpl() + MockitoAnnotations.openMocks(this) + testee = SpecialUrlDetectorImpl(mockContext, mockSettingsDataStore) + whenever(mockContext.packageManager).thenReturn(mockPackageManager) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(emptyList()) + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) } @Test @@ -59,6 +86,59 @@ class SpecialUrlDetectorImplTest { assertEquals("https://example.com", type.webAddress) } + @Test + fun whenAppLinksEnabledAndNoNonBrowserActivitiesFoundThenReturnWebType() { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildBrowserResolveInfo())) + val type = testee.determineType("https://example.com") + assertTrue(type is Web) + } + + @Test + fun whenAppLinksDisabledAndNonBrowserActivityFoundThenReturnWebType() { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(false) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo())) + val type = testee.determineType("https://example.com") + assertTrue(type is Web) + } + + @Test + fun whenURISyntaxExceptionThrownThenReturnWebType() { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + given(mockPackageManager.queryIntentActivities(any(), anyInt())).willAnswer { throw URISyntaxException("", "") } + val type = testee.determineType("https://example.com") + assertTrue(type is Web) + } + + @Test + fun whenAppLinksEnabledAndOneNonBrowserActivityFoundThenReturnAppLink() { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) + val type = testee.determineType("https://example.com") + verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) + assertTrue(type is AppLink) + val appLinkType = type as AppLink + assertEquals("https://example.com", appLinkType.url) + assertEquals(EXAMPLE_APP_PACKAGE, appLinkType.appIntent!!.component!!.packageName) + assertEquals(EXAMPLE_APP_ACTIVITY_NAME, appLinkType.appIntent!!.component!!.className) + assertNull(appLinkType.excludedComponents) + } + + @Test + fun whenAppLinksEnabledAndMultipleNonBrowserActivitiesFoundThenReturnAppLink() { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) + val type = testee.determineType("https://example.com") + verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) + assertTrue(type is AppLink) + val appLinkType = type as AppLink + assertEquals("https://example.com", appLinkType.url) + assertEquals(1, appLinkType.excludedComponents!!.size) + assertEquals(EXAMPLE_BROWSER_PACKAGE, appLinkType.excludedComponents!![0].packageName) + assertEquals(EXAMPLE_BROWSER_ACTIVITY_NAME, appLinkType.excludedComponents!![0].className) + assertNull(appLinkType.appIntent) + } + @Test fun whenUrlIsTelWithDashesThenTelephoneTypeDetected() { val expected = Telephone::class @@ -132,8 +212,8 @@ class SpecialUrlDetectorImplTest { } @Test - fun whenUrlIsCustomUriSchemeThenIntentTypeDetected() { - val type = testee.determineType("myapp:foo bar") as IntentType + fun whenUrlIsCustomUriSchemeThenNonHttpAppLinkTypeDetected() { + val type = testee.determineType("myapp:foo bar") as NonHttpAppLink assertEquals("myapp:foo bar", type.url) } @@ -195,4 +275,31 @@ class SpecialUrlDetectorImplTest { val charList: List = ('a'..'z') + ('0'..'9') return List(length) { charList.random() }.joinToString("") } + + private fun buildAppResolveInfo(): ResolveInfo { + val activity = ResolveInfo() + activity.filter = IntentFilter() + activity.filter.addDataAuthority("host.com", "123") + activity.filter.addDataPath("/path", 0) + activity.activityInfo = ActivityInfo() + activity.activityInfo.packageName = EXAMPLE_APP_PACKAGE + activity.activityInfo.name = EXAMPLE_APP_ACTIVITY_NAME + return activity + } + + private fun buildBrowserResolveInfo(): ResolveInfo { + val activity = ResolveInfo() + activity.filter = IntentFilter() + activity.activityInfo = ActivityInfo() + activity.activityInfo.packageName = EXAMPLE_BROWSER_PACKAGE + activity.activityInfo.name = EXAMPLE_BROWSER_ACTIVITY_NAME + return activity + } + + companion object { + const val EXAMPLE_APP_PACKAGE = "com.test.apppackage" + const val EXAMPLE_APP_ACTIVITY_NAME = "com.test.AppActivity" + const val EXAMPLE_BROWSER_PACKAGE = "com.test.browserpackage" + const val EXAMPLE_BROWSER_ACTIVITY_NAME = "com.test.BrowserActivity" + } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt index ef25c8a8ee7f..0f349c7c06e0 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt @@ -191,6 +191,18 @@ class SettingsViewModelTest { verify(mockAppSettingsDataStore).autoCompleteSuggestionsEnabled = false } + @Test + fun whenAppLinksSwitchedOnThenDataStoreIsUpdated() { + testee.onAppLinksSettingChanged(true) + verify(mockAppSettingsDataStore).appLinksEnabled = true + } + + @Test + fun whenAppLinksSwitchedOffThenDataStoreIsUpdated() { + testee.onAppLinksSettingChanged(false) + verify(mockAppSettingsDataStore).appLinksEnabled = false + } + @Test fun whenLeaveFeedBackRequestedThenCommandIsLaunchFeedback() = coroutineTestRule.runBlocking { testee.commands().test { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 1b28cf825d9e..0536d033c085 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -26,10 +26,7 @@ import android.content.pm.PackageManager import android.content.res.Configuration import android.media.MediaScannerConnection import android.net.Uri -import android.os.Bundle -import android.os.Environment -import android.os.Handler -import android.os.Message +import android.os.* import android.provider.Settings import android.text.Editable import android.view.* @@ -46,6 +43,7 @@ import android.widget.EditText import android.widget.TextView import android.widget.Toast import androidx.annotation.AnyThread +import androidx.annotation.RequiresApi import androidx.annotation.StringRes import androidx.appcompat.app.AlertDialog import androidx.core.app.ActivityCompat @@ -629,8 +627,11 @@ class BrowserTabFragment : addHomeShortcut(it, context) } } - is Command.HandleExternalAppLink -> { - openExternalDialog(it.appLink.intent, it.appLink.fallbackUrl, false, it.headers) + is Command.HandleAppLink -> { + openAppLinkDialog(it.appLink.appIntent, it.appLink.excludedComponents, it.appLink.url, it.headers) + } + is Command.HandleNonHttpAppLink -> { + openExternalDialog(it.nonHttpAppLink.intent, it.nonHttpAppLink.fallbackUrl, false, it.headers) } is Command.LaunchSurvey -> launchSurvey(it.survey) is Command.LaunchAddWidget -> launchAddWidget() @@ -801,6 +802,31 @@ class BrowserTabFragment : decorator.incrementTabs() } + private fun openAppLinkDialog( + appIntent: Intent?, + excludedComponents: List?, + url: String? = null, + headers: Map = emptyMap() + ) { + context?.let { + if (appIntent != null) { + launchAppLinkDialog(it, url, headers) { it.startActivity(appIntent) } + } else if (excludedComponents != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + val title = getString(R.string.openExternalApp) + val chooserIntent = getChooserIntent(url, title, excludedComponents) + launchAppLinkDialog(it, url, headers) { it.startActivity(chooserIntent) } + } + } + } + + @RequiresApi(Build.VERSION_CODES.N) + private fun getChooserIntent(url: String?, title: String, excludedComponents: List): Intent { + val urlIntent = Intent.parseUri(url, 0) + val chooserIntent = Intent.createChooser(urlIntent, title) + chooserIntent.putExtra(Intent.EXTRA_EXCLUDE_COMPONENTS, excludedComponents.toTypedArray()) + return chooserIntent + } + private fun openExternalDialog( intent: Intent, fallbackUrl: String? = null, @@ -893,6 +919,34 @@ class BrowserTabFragment : } } + private fun launchAppLinkDialog(context: Context, url: String?, headers: Map, onClick: () -> Unit) { + val isShowing = alertDialog?.isShowing + + if (isShowing != true) { + alertDialog = AlertDialog.Builder(context) + .setTitle(R.string.launchingAssociatedApp) + .setMessage(getString(R.string.confirmOpenAssociatedApp)) + .setPositiveButton(R.string.yes) { _, _ -> + onClick() + } + .setNeutralButton(R.string.closeTab) { dialog, _ -> + dialog.dismiss() + launch { + viewModel.closeCurrentTab() + destroyWebView() + } + } + .setNegativeButton(R.string.no) { _, _ -> + if (url != null) { + webView?.loadUrl(url, headers) + } else { + showToast(R.string.unableToOpenLink) + } + } + .show() + } + } + override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { if (requestCode == REQUEST_CODE_CHOOSE_FILE) { handleFileUploadResult(resultCode, data) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index ef23dfeb6752..1beddb7e9a7b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -49,7 +49,8 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.Command.* import com.duckduckgo.app.browser.BrowserTabViewModel.GlobalLayoutViewState.Browser import com.duckduckgo.app.browser.BrowserTabViewModel.GlobalLayoutViewState.Invalidated import com.duckduckgo.app.browser.LongPressHandler.RequiredAction -import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.IntentType +import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.AppLink +import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.NonHttpAppLink import com.duckduckgo.app.browser.WebNavigationStateChange.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector import com.duckduckgo.app.browser.downloader.DownloadFailReason @@ -278,7 +279,8 @@ class BrowserTabViewModel( class BrokenSiteFeedback(val data: BrokenSiteData) : Command() object DismissFindInPage : Command() class ShowFileChooser(val filePathCallback: ValueCallback>, val fileChooserParams: WebChromeClient.FileChooserParams) : Command() - class HandleExternalAppLink(val appLink: IntentType, val headers: Map) : Command() + class HandleNonHttpAppLink(val nonHttpAppLink: NonHttpAppLink, val headers: Map) : Command() + class HandleAppLink(val appLink: AppLink, val headers: Map) : Command() class AddHomeShortcut(val title: String, val url: String, val icon: Bitmap? = null) : Command() class LaunchSurvey(val survey: Survey) : Command() object LaunchAddWidget : Command() @@ -568,8 +570,8 @@ class BrowserTabViewModel( val urlToNavigate = queryUrlConverter.convertQueryToUrl(trimmedInput, verticalParameter, queryOrigin) val type = specialUrlDetector.determineType(trimmedInput) - if (type is IntentType) { - externalAppLinkClicked(type) + if (type is NonHttpAppLink) { + nonHttpAppLinkClicked(type) } else { if (shouldClearHistoryOnNewQuery()) { command.value = ResetHistory @@ -1752,8 +1754,12 @@ class BrowserTabViewModel( tabRepository.updateTabPreviewImage(tabId, null) } - override fun externalAppLinkClicked(appLink: IntentType) { - command.value = HandleExternalAppLink(appLink, getUrlHeaders()) + override fun appLinkClicked(appLink: AppLink) { + command.value = HandleAppLink(appLink, getUrlHeaders()) + } + + override fun nonHttpAppLinkClicked(appLink: NonHttpAppLink) { + command.value = HandleNonHttpAppLink(appLink, getUrlHeaders()) } override fun openMessageInNewTab(message: Message) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 64b33fd935ea..3d942721b8dd 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -68,9 +68,10 @@ class BrowserWebViewClient( /** * This is the new method of url overriding available from API 24 onwards */ + @RequiresApi(Build.VERSION_CODES.N) override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean { val url = request.url - return shouldOverride(view, url, request.isForMainFrame) + return shouldOverride(view, url, request.isForMainFrame, request.isRedirect) } /** @@ -79,13 +80,13 @@ class BrowserWebViewClient( @Suppress("OverridingDeprecatedMember") override fun shouldOverrideUrlLoading(view: WebView, urlString: String): Boolean { val url = Uri.parse(urlString) - return shouldOverride(view, url, true) + return shouldOverride(view, url, isForMainFrame = true, isRedirect = false) } /** * API-agnostic implementation of deciding whether to override url or not */ - private fun shouldOverride(webView: WebView, url: Uri, isForMainFrame: Boolean): Boolean { + private fun shouldOverride(webView: WebView, url: Uri, isForMainFrame: Boolean, isRedirect: Boolean): Boolean { Timber.v("shouldOverride $url") try { @@ -108,7 +109,14 @@ class BrowserWebViewClient( webViewClientListener?.sendSmsRequested(urlType.telephoneNumber) true } - is SpecialUrlDetector.UrlType.IntentType -> { + is SpecialUrlDetector.UrlType.AppLink -> { + if (isRedirect) { + return false + } + launchExternalApp(urlType) + true + } + is SpecialUrlDetector.UrlType.NonHttpAppLink -> { Timber.i("Found intent type link for $urlType.url") launchExternalApp(urlType) true @@ -142,8 +150,12 @@ class BrowserWebViewClient( } } - private fun launchExternalApp(urlType: SpecialUrlDetector.UrlType.IntentType) { - webViewClientListener?.externalAppLinkClicked(urlType) + private fun launchExternalApp(urlType: SpecialUrlDetector.UrlType.NonHttpAppLink) { + webViewClientListener?.nonHttpAppLinkClicked(urlType) + } + + private fun launchExternalApp(urlType: SpecialUrlDetector.UrlType.AppLink) { + webViewClientListener?.appLinkClicked(urlType) } @UiThread diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 64117744fe8e..e7a2c2be73e8 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -16,9 +16,15 @@ package com.duckduckgo.app.browser +import android.content.ComponentName +import android.content.Context import android.content.Intent +import android.content.pm.PackageManager +import android.content.pm.ResolveInfo import android.net.Uri +import android.os.Build import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType +import com.duckduckgo.app.settings.db.SettingsDataStore import timber.log.Timber import java.net.URISyntaxException @@ -31,14 +37,17 @@ interface SpecialUrlDetector { class Telephone(val telephoneNumber: String) : UrlType() class Email(val emailAddress: String) : UrlType() class Sms(val telephoneNumber: String) : UrlType() - class IntentType(val url: String, val intent: Intent, val fallbackUrl: String?) : UrlType() + class AppLink(val appIntent: Intent? = null, val excludedComponents: List? = null, val url: String? = null) : UrlType() + class NonHttpAppLink(val url: String, val intent: Intent, val fallbackUrl: String?) : UrlType() class SearchQuery(val query: String) : UrlType() class Unknown(val url: String) : UrlType() } - } -class SpecialUrlDetectorImpl : SpecialUrlDetector { +class SpecialUrlDetectorImpl( + private val context: Context, + private val settingsDataStore: SettingsDataStore +) : SpecialUrlDetector { override fun determineType(uri: Uri): UrlType { val uriString = uri.toString() @@ -49,7 +58,7 @@ class SpecialUrlDetectorImpl : SpecialUrlDetector { MAILTO_SCHEME -> buildEmail(uriString) SMS_SCHEME -> buildSms(uriString) SMSTO_SCHEME -> buildSmsTo(uriString) - HTTP_SCHEME, HTTPS_SCHEME, DATA_SCHEME -> UrlType.Web(uriString) + HTTP_SCHEME, HTTPS_SCHEME, DATA_SCHEME -> checkForAppLink(uriString) ABOUT_SCHEME -> UrlType.Unknown(uriString) JAVASCRIPT_SCHEME -> UrlType.SearchQuery(uriString) null -> UrlType.SearchQuery(uriString) @@ -68,6 +77,52 @@ class SpecialUrlDetectorImpl : SpecialUrlDetector { private fun buildSmsTo(uriString: String): UrlType = UrlType.Sms(uriString.removePrefix("$SMSTO_SCHEME:").truncate(SMS_MAX_LENGTH)) + private fun checkForAppLink(uriString: String): UrlType { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && settingsDataStore.appLinksEnabled) { + try { + val activities = queryActivities(uriString) + val nonBrowserActivities = keepNonBrowserActivities(activities) + + if (nonBrowserActivities.isNotEmpty()) { + nonBrowserActivities.singleOrNull()?.let { resolveInfo -> + val nonBrowserIntent = buildNonBrowserIntent(resolveInfo, uriString) + return UrlType.AppLink(appIntent = nonBrowserIntent, url = uriString) + } + val excludedComponents = getExcludedComponents(activities) + return UrlType.AppLink(excludedComponents = excludedComponents, url = uriString) + } + } catch (e: URISyntaxException) { + Timber.w(e, "Failed to parse uri $uriString") + } + } + return UrlType.Web(uriString) + } + + @Throws(URISyntaxException::class) + private fun queryActivities(uriString: String): MutableList { + val browsableIntent = Intent.parseUri(uriString, 0) + browsableIntent.addCategory(Intent.CATEGORY_BROWSABLE) + return context.packageManager.queryIntentActivities(browsableIntent, PackageManager.GET_RESOLVED_FILTER) + } + + private fun keepNonBrowserActivities(activities: List): List { + return activities.filter { resolveInfo -> + resolveInfo.filter != null && !(resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0) + } + } + @Throws(URISyntaxException::class) + private fun buildNonBrowserIntent(nonBrowserActivity: ResolveInfo, uriString: String): Intent { + val intent = Intent.parseUri(uriString, 0) + intent.component = ComponentName(nonBrowserActivity.activityInfo.packageName, nonBrowserActivity.activityInfo.name) + return intent + } + + private fun getExcludedComponents(activities: List): List { + return activities.filter { resolveInfo -> + resolveInfo.filter != null && (resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0) + }.map { ComponentName(it.activityInfo.packageName, it.activityInfo.name) } + } + private fun checkForIntent(scheme: String, uriString: String): UrlType { val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+") if (scheme.matches(validUriSchemeRegex)) { @@ -81,7 +136,7 @@ class SpecialUrlDetectorImpl : SpecialUrlDetector { return try { val intent = Intent.parseUri(uriString, 0) val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL) - UrlType.IntentType(url = uriString, intent = intent, fallbackUrl = fallbackUrl) + UrlType.NonHttpAppLink(url = uriString, intent = intent, fallbackUrl = fallbackUrl) } catch (e: URISyntaxException) { Timber.w(e, "Failed to parse uri $uriString") return UrlType.Unknown(uriString) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt index 5b6596c3e238..cb956c8a7974 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt @@ -47,7 +47,8 @@ interface WebViewClientListener { fun goFullScreen(view: View) fun exitFullScreen() fun showFileChooser(filePathCallback: ValueCallback>, fileChooserParams: WebChromeClient.FileChooserParams) - fun externalAppLinkClicked(appLink: SpecialUrlDetector.UrlType.IntentType) + fun nonHttpAppLinkClicked(appLink: SpecialUrlDetector.UrlType.NonHttpAppLink) + fun appLinkClicked(appLink: SpecialUrlDetector.UrlType.AppLink) fun openMessageInNewTab(message: Message) fun recoverFromRenderProcessGone() fun requiresAuthentication(request: BasicAuthenticationRequest) diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 4a9b6800c499..abecd35bc749 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -174,7 +174,7 @@ class BrowserModule { } @Provides - fun specialUrlDetector(): SpecialUrlDetector = SpecialUrlDetectorImpl() + fun specialUrlDetector(context: Context, settingsDataStore: SettingsDataStore): SpecialUrlDetector = SpecialUrlDetectorImpl(context, settingsDataStore) @Provides @Singleton diff --git a/app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt b/app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt index 4c7bfe9ba50d..51d9fdc0369f 100644 --- a/app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt @@ -80,12 +80,17 @@ class SettingsActivity : viewModel.onAutocompleteSettingChanged(isChecked) } + private val appLinksToggleListener = OnCheckedChangeListener { _, isChecked -> + viewModel.onAppLinksSettingChanged(isChecked) + } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_settings) setupToolbar(toolbar) configureUiEventHandlers() + configureAppLinksToggle() observeViewModel() } @@ -112,6 +117,14 @@ class SettingsActivity : emailSetting.setOnClickListener { viewModel.onEmailSettingClicked() } } + private fun configureAppLinksToggle() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + appLinksToggle.setOnCheckedChangeListener(appLinksToggleListener) + } else { + appLinksToggle.visibility = View.GONE + } + } + private fun observeViewModel() { viewModel.viewState() .flowWithLifecycle(lifecycle, Lifecycle.State.CREATED) @@ -126,6 +139,7 @@ class SettingsActivity : changeAppIcon.setImageResource(it.appIcon.icon) updateSelectedFireAnimation(it.selectedFireAnimation) setEmailSetting(it.emailSetting) + appLinksToggle.quietlySetIsChecked(it.appLinksEnabled, appLinksToggleListener) } }.launchIn(lifecycleScope) diff --git a/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt b/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt index 5fba0ec4d381..74704ac2be2e 100644 --- a/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt @@ -65,7 +65,8 @@ class SettingsViewModel @Inject constructor( val automaticallyClearData: AutomaticallyClearData = AutomaticallyClearData(ClearWhatOption.CLEAR_NONE, ClearWhenOption.APP_EXIT_ONLY), val appIcon: AppIcon = AppIcon.DEFAULT, val globalPrivacyControlEnabled: Boolean = false, - val emailSetting: EmailSetting = EmailSetting.EmailSettingOff + val emailSetting: EmailSetting = EmailSetting.EmailSettingOff, + val appLinksEnabled: Boolean = true ) sealed class EmailSetting { @@ -122,7 +123,8 @@ class SettingsViewModel @Inject constructor( appIcon = settingsDataStore.appIcon, selectedFireAnimation = settingsDataStore.selectedFireAnimation, globalPrivacyControlEnabled = settingsDataStore.globalPrivacyControlEnabled, - emailSetting = getEmailSetting() + emailSetting = getEmailSetting(), + appLinksEnabled = settingsDataStore.appLinksEnabled ) ) } @@ -211,6 +213,12 @@ class SettingsViewModel @Inject constructor( viewModelScope.launch { viewState.emit(currentViewState().copy(autoCompleteSuggestionsEnabled = enabled)) } } + fun onAppLinksSettingChanged(enabled: Boolean) { + Timber.i("User changed app links setting, is now enabled: $enabled") + settingsDataStore.appLinksEnabled = enabled + viewModelScope.launch { viewState.emit(currentViewState().copy(appLinksEnabled = enabled)) } + } + private fun obtainVersion(variantKey: String): String { val formattedVariantKey = if (variantKey.isBlank()) " " else " $variantKey " return "${BuildConfig.VERSION_NAME}$formattedVariantKey(${BuildConfig.VERSION_CODE})" diff --git a/app/src/main/java/com/duckduckgo/app/settings/db/SettingsDataStore.kt b/app/src/main/java/com/duckduckgo/app/settings/db/SettingsDataStore.kt index 05a8456f6aed..2b6071079472 100644 --- a/app/src/main/java/com/duckduckgo/app/settings/db/SettingsDataStore.kt +++ b/app/src/main/java/com/duckduckgo/app/settings/db/SettingsDataStore.kt @@ -41,6 +41,7 @@ interface SettingsDataStore { var appLocationPermission: Boolean var appLocationPermissionDeniedForever: Boolean var globalPrivacyControlEnabled: Boolean + var appLinksEnabled: Boolean /** * This will be checked upon app startup and used to decide whether it should perform a clear or not. @@ -146,6 +147,10 @@ class SettingsSharedPreferences constructor(private val context: Context, privat get() = preferences.getBoolean(KEY_DO_NOT_SELL_ENABLED, true) set(enabled) = preferences.edit { putBoolean(KEY_DO_NOT_SELL_ENABLED, enabled) } + override var appLinksEnabled: Boolean + get() = preferences.getBoolean(APP_LINKS_ENABLED, true) + set(enabled) = preferences.edit { putBoolean(APP_LINKS_ENABLED, enabled) } + override fun hasBackgroundTimestampRecorded(): Boolean = preferences.contains(KEY_APP_BACKGROUNDED_TIMESTAMP) override fun clearAppBackgroundTimestamp() = preferences.edit { remove(KEY_APP_BACKGROUNDED_TIMESTAMP) } @@ -199,6 +204,7 @@ class SettingsSharedPreferences constructor(private val context: Context, privat const val KEY_SITE_LOCATION_PERMISSION_ENABLED = "KEY_SITE_LOCATION_PERMISSION_ENABLED" const val KEY_SYSTEM_LOCATION_PERMISSION_DENIED_FOREVER = "KEY_SYSTEM_LOCATION_PERMISSION_DENIED_FOREVER" const val KEY_DO_NOT_SELL_ENABLED = "KEY_DO_NOT_SELL_ENABLED" + const val APP_LINKS_ENABLED = "APP_LINKS_ENABLED" private val DEFAULT_ICON = if (BuildConfig.DEBUG) { AppIcon.BLUE diff --git a/app/src/main/res/layout/content_settings_privacy.xml b/app/src/main/res/layout/content_settings_privacy.xml index 0bbbaf025267..068f8abdd35c 100644 --- a/app/src/main/res/layout/content_settings_privacy.xml +++ b/app/src/main/res/layout/content_settings_privacy.xml @@ -89,4 +89,14 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/whitelist" /> + + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 14c05dfef1d9..4a1343c009b7 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -65,6 +65,12 @@ Would you like to leave DuckDuckGo to view this content? Unable to open this type of link + + Open in associated app? + Would you like to leave DuckDuckGo to view this content in another app? + Open Links in Associated Apps + Disable to prevent links from automatically opening in other installed apps. + DuckDuckGo Tabs From 9f87e3189248a7d0bd357babea91d90a887f301f Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Wed, 2 Jun 2021 11:19:08 +0200 Subject: [PATCH 02/26] Fix ktLint issues --- .../app/browser/BrowserTabFragment.kt | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 0536d033c085..ce811c95eb61 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -803,10 +803,10 @@ class BrowserTabFragment : } private fun openAppLinkDialog( - appIntent: Intent?, - excludedComponents: List?, - url: String? = null, - headers: Map = emptyMap() + appIntent: Intent?, + excludedComponents: List?, + url: String? = null, + headers: Map = emptyMap() ) { context?.let { if (appIntent != null) { @@ -924,26 +924,26 @@ class BrowserTabFragment : if (isShowing != true) { alertDialog = AlertDialog.Builder(context) - .setTitle(R.string.launchingAssociatedApp) - .setMessage(getString(R.string.confirmOpenAssociatedApp)) - .setPositiveButton(R.string.yes) { _, _ -> - onClick() - } - .setNeutralButton(R.string.closeTab) { dialog, _ -> - dialog.dismiss() - launch { - viewModel.closeCurrentTab() - destroyWebView() - } + .setTitle(R.string.launchingAssociatedApp) + .setMessage(getString(R.string.confirmOpenAssociatedApp)) + .setPositiveButton(R.string.yes) { _, _ -> + onClick() + } + .setNeutralButton(R.string.closeTab) { dialog, _ -> + dialog.dismiss() + launch { + viewModel.closeCurrentTab() + destroyWebView() } - .setNegativeButton(R.string.no) { _, _ -> - if (url != null) { - webView?.loadUrl(url, headers) - } else { - showToast(R.string.unableToOpenLink) - } + } + .setNegativeButton(R.string.no) { _, _ -> + if (url != null) { + webView?.loadUrl(url, headers) + } else { + showToast(R.string.unableToOpenLink) } - .show() + } + .show() } } From 51bab21e72a6a351d96f55c7ec0a00dcb059035b Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Wed, 2 Jun 2021 12:54:16 +0200 Subject: [PATCH 03/26] Update tests for API <24 --- .../app/browser/BrowserWebViewClientTest.kt | 35 +++++++---- .../app/browser/SpecialUrlDetectorImplTest.kt | 58 +++++++++++++------ .../app/browser/BrowserWebViewClient.kt | 2 +- 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 8056609f37ba..f4f4634a1e09 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -71,7 +71,6 @@ class BrowserWebViewClientTest { private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val emailInjector: EmailInjector = mock() private val webResourceRequest: WebResourceRequest = mock() - private val webViewClientListener: WebViewClientListener = mock() @UiThreadTest @Before @@ -267,21 +266,33 @@ class BrowserWebViewClientTest { verify(uncaughtExceptionRepository).recordUncaughtException(exception, UncaughtExceptionSource.SHOULD_OVERRIDE_REQUEST) } - @Test - fun whenAppLinkDetectedAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { - whenever(webResourceRequest.isRedirect).thenReturn(true) - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) - assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - } - @Test fun whenAppLinkDetectedThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { - whenever(webResourceRequest.isRedirect).thenReturn(false) val urlType = SpecialUrlDetector.UrlType.AppLink() whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) - testee.webViewClientListener = webViewClientListener - assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(webViewClientListener).appLinkClicked(urlType) + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(webResourceRequest.isRedirect).thenReturn(false) + assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener).appLinkClicked(urlType) + } else { + assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) + verify(listener, never()).appLinkClicked(any()) + } + } + + @Test + fun whenAppLinkDetectedAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(webResourceRequest.isRedirect).thenReturn(true) + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener, never()).appLinkClicked(any()) + } else { + assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) + verify(listener, never()).appLinkClicked(any()) + } } private class TestWebView(context: Context) : WebView(context) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index 8a96f4f1b4f8..04ba43e053fe 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -22,6 +22,7 @@ import android.content.IntentFilter import android.content.pm.ActivityInfo import android.content.pm.PackageManager import android.content.pm.ResolveInfo +import android.os.Build import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.* import com.duckduckgo.app.browser.SpecialUrlDetectorImpl.Companion.EMAIL_MAX_LENGTH import com.duckduckgo.app.browser.SpecialUrlDetectorImpl.Companion.PHONE_MAX_LENGTH @@ -91,52 +92,71 @@ class SpecialUrlDetectorImplTest { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildBrowserResolveInfo())) val type = testee.determineType("https://example.com") + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) + } else { + verifyZeroInteractions(mockPackageManager) + } assertTrue(type is Web) } @Test - fun whenAppLinksDisabledAndNonBrowserActivityFoundThenReturnWebType() { + fun whenAppLinksDisabledThenReturnWebType() { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(false) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo())) val type = testee.determineType("https://example.com") + verifyZeroInteractions(mockPackageManager) assertTrue(type is Web) } @Test - fun whenURISyntaxExceptionThrownThenReturnWebType() { + fun whenAppLinkThrowsURISyntaxExceptionThenReturnWebType() { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) given(mockPackageManager.queryIntentActivities(any(), anyInt())).willAnswer { throw URISyntaxException("", "") } val type = testee.determineType("https://example.com") + verifyZeroInteractions(mockPackageManager) assertTrue(type is Web) } @Test - fun whenAppLinksEnabledAndOneNonBrowserActivityFoundThenReturnAppLink() { + fun whenAppLinksEnabledAndOneNonBrowserActivityFoundThenReturnAppLinkWithIntent() { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) val type = testee.determineType("https://example.com") - verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) - assertTrue(type is AppLink) - val appLinkType = type as AppLink - assertEquals("https://example.com", appLinkType.url) - assertEquals(EXAMPLE_APP_PACKAGE, appLinkType.appIntent!!.component!!.packageName) - assertEquals(EXAMPLE_APP_ACTIVITY_NAME, appLinkType.appIntent!!.component!!.className) - assertNull(appLinkType.excludedComponents) + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) + assertTrue(type is AppLink) + val appLinkType = type as AppLink + assertEquals("https://example.com", appLinkType.url) + assertEquals(EXAMPLE_APP_PACKAGE, appLinkType.appIntent!!.component!!.packageName) + assertEquals(EXAMPLE_APP_ACTIVITY_NAME, appLinkType.appIntent!!.component!!.className) + assertNull(appLinkType.excludedComponents) + } else { + verifyZeroInteractions(mockPackageManager) + assertTrue(type is Web) + } } @Test - fun whenAppLinksEnabledAndMultipleNonBrowserActivitiesFoundThenReturnAppLink() { + fun whenAppLinksEnabledAndMultipleNonBrowserActivitiesFoundThenReturnAppLinkWithExcludedComponents() { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) val type = testee.determineType("https://example.com") - verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) - assertTrue(type is AppLink) - val appLinkType = type as AppLink - assertEquals("https://example.com", appLinkType.url) - assertEquals(1, appLinkType.excludedComponents!!.size) - assertEquals(EXAMPLE_BROWSER_PACKAGE, appLinkType.excludedComponents!![0].packageName) - assertEquals(EXAMPLE_BROWSER_ACTIVITY_NAME, appLinkType.excludedComponents!![0].className) - assertNull(appLinkType.appIntent) + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) + assertTrue(type is AppLink) + val appLinkType = type as AppLink + assertEquals("https://example.com", appLinkType.url) + assertEquals(1, appLinkType.excludedComponents!!.size) + assertEquals(EXAMPLE_BROWSER_PACKAGE, appLinkType.excludedComponents!![0].packageName) + assertEquals(EXAMPLE_BROWSER_ACTIVITY_NAME, appLinkType.excludedComponents!![0].className) + assertNull(appLinkType.appIntent) + } else { + verifyZeroInteractions(mockPackageManager) + assertTrue(type is Web) + } } @Test diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 3d942721b8dd..4a208453249c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -80,7 +80,7 @@ class BrowserWebViewClient( @Suppress("OverridingDeprecatedMember") override fun shouldOverrideUrlLoading(view: WebView, urlString: String): Boolean { val url = Uri.parse(urlString) - return shouldOverride(view, url, isForMainFrame = true, isRedirect = false) + return shouldOverride(view, url, isForMainFrame = true, isRedirect = true) } /** From babaad18ffb63f0549e48cc06449929d891d9da0 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Wed, 2 Jun 2021 13:53:57 +0200 Subject: [PATCH 04/26] Fix broken tests --- .../duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index 04ba43e053fe..905d6d55721c 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -92,11 +92,6 @@ class SpecialUrlDetectorImplTest { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildBrowserResolveInfo())) val type = testee.determineType("https://example.com") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) - } else { - verifyZeroInteractions(mockPackageManager) - } assertTrue(type is Web) } @@ -114,7 +109,6 @@ class SpecialUrlDetectorImplTest { whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) given(mockPackageManager.queryIntentActivities(any(), anyInt())).willAnswer { throw URISyntaxException("", "") } val type = testee.determineType("https://example.com") - verifyZeroInteractions(mockPackageManager) assertTrue(type is Web) } From f436a59b2a2283a5dab2fdeef5342326b982a34f Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Thu, 3 Jun 2021 11:23:19 +0200 Subject: [PATCH 05/26] Remove close tab button from app link dialog --- .../app/browser/BrowserWebViewClientTest.kt | 32 +++++++++++++------ .../app/browser/BrowserTabFragment.kt | 7 ---- .../app/browser/BrowserWebViewClient.kt | 2 +- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index f4f4634a1e09..e0dc2200e9ed 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -268,28 +268,42 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { - val urlType = SpecialUrlDetector.UrlType.AppLink() - whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + val urlType = SpecialUrlDetector.UrlType.AppLink() + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) + whenever(webResourceRequest.isForMainFrame).thenReturn(true) assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener).appLinkClicked(urlType) - } else { - assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) - verify(listener, never()).appLinkClicked(any()) } } @Test fun whenAppLinkDetectedAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) whenever(webResourceRequest.isRedirect).thenReturn(true) + whenever(webResourceRequest.isForMainFrame).thenReturn(true) + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener, never()).appLinkClicked(any()) + } + } + + @Test + fun whenAppLinkDetectedAndIsNotForMainFrameThenReturnFalse() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + whenever(webResourceRequest.isRedirect).thenReturn(false) + whenever(webResourceRequest.isForMainFrame).thenReturn(false) assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener, never()).appLinkClicked(any()) - } else { + } + } + + @Test + fun whenAppLinkDetectedOnApiLessThan24ThenReturnFalse() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) verify(listener, never()).appLinkClicked(any()) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index ce811c95eb61..9b5c04f0e4d7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -929,13 +929,6 @@ class BrowserTabFragment : .setPositiveButton(R.string.yes) { _, _ -> onClick() } - .setNeutralButton(R.string.closeTab) { dialog, _ -> - dialog.dismiss() - launch { - viewModel.closeCurrentTab() - destroyWebView() - } - } .setNegativeButton(R.string.no) { _, _ -> if (url != null) { webView?.loadUrl(url, headers) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 4a208453249c..a3a3a0c5ac3c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -110,7 +110,7 @@ class BrowserWebViewClient( true } is SpecialUrlDetector.UrlType.AppLink -> { - if (isRedirect) { + if (isRedirect || !isForMainFrame) { return false } launchExternalApp(urlType) From 6be2222f3af9d9c3329029ae6d0354198f532225 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Mon, 7 Jun 2021 14:14:00 +0200 Subject: [PATCH 06/26] Handle redirects after an app link has been triggered --- .../duckduckgo/app/browser/BrowserWebViewClientTest.kt | 4 ++-- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 ++ .../com/duckduckgo/app/browser/BrowserWebViewClient.kt | 10 ++++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index e0dc2200e9ed..be5410dd4338 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -45,7 +45,6 @@ import kotlinx.coroutines.test.TestCoroutineScope import org.junit.Before import org.junit.Rule import org.junit.Test -import java.lang.RuntimeException @ExperimentalCoroutinesApi class BrowserWebViewClientTest { @@ -279,11 +278,12 @@ class BrowserWebViewClientTest { } @Test - fun whenAppLinkDetectedAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { + fun whenAppLinkTriggeredAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) whenever(webResourceRequest.isRedirect).thenReturn(true) whenever(webResourceRequest.isForMainFrame).thenReturn(true) + testee.appLinkTriggered = true assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener, never()).appLinkClicked(any()) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 9b5c04f0e4d7..b6e226a2bb24 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -928,10 +928,12 @@ class BrowserTabFragment : .setMessage(getString(R.string.confirmOpenAssociatedApp)) .setPositiveButton(R.string.yes) { _, _ -> onClick() + webViewClient.appLinkTriggered = false } .setNegativeButton(R.string.no) { _, _ -> if (url != null) { webView?.loadUrl(url, headers) + webViewClient.appLinkTriggered = true } else { showToast(R.string.unableToOpenLink) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index a3a3a0c5ac3c..e20710969a08 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -64,6 +64,7 @@ class BrowserWebViewClient( var webViewClientListener: WebViewClientListener? = null private var lastPageStarted: String? = null + var appLinkTriggered = false /** * This is the new method of url overriding available from API 24 onwards @@ -110,14 +111,18 @@ class BrowserWebViewClient( true } is SpecialUrlDetector.UrlType.AppLink -> { - if (isRedirect || !isForMainFrame) { + Timber.i("Found app link for ${urlType.url}") + if (isRedirect && appLinkTriggered || !isForMainFrame) { return false } launchExternalApp(urlType) true } is SpecialUrlDetector.UrlType.NonHttpAppLink -> { - Timber.i("Found intent type link for $urlType.url") + Timber.i("Found intent type link for ${urlType.url}") + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && isRedirect && appLinkTriggered) { + return true + } launchExternalApp(urlType) true } @@ -176,6 +181,7 @@ class BrowserWebViewClient( emailInjector.resetInjectedJsFlag() globalPrivacyControl.injectDoNotSellToDom(webView) loginDetector.onEvent(WebNavigationEvent.OnPageStarted(webView)) + appLinkTriggered = false } catch (e: Throwable) { appCoroutineScope.launch { uncaughtExceptionRepository.recordUncaughtException(e, ON_PAGE_STARTED) From 01d65342d1d3a83ffed7389dcdcd3b0cc7e67e60 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Mon, 7 Jun 2021 14:35:02 +0200 Subject: [PATCH 07/26] Use existing copy for app link dialog title and message --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- app/src/main/res/values/strings.xml | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index b6e226a2bb24..98bc8cd87ee0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -924,8 +924,8 @@ class BrowserTabFragment : if (isShowing != true) { alertDialog = AlertDialog.Builder(context) - .setTitle(R.string.launchingAssociatedApp) - .setMessage(getString(R.string.confirmOpenAssociatedApp)) + .setTitle(R.string.launchingExternalApp) + .setMessage(getString(R.string.confirmOpenExternalApp)) .setPositiveButton(R.string.yes) { _, _ -> onClick() webViewClient.appLinkTriggered = false diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4a1343c009b7..c4c89c0fffaa 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -66,8 +66,6 @@ Unable to open this type of link - Open in associated app? - Would you like to leave DuckDuckGo to view this content in another app? Open Links in Associated Apps Disable to prevent links from automatically opening in other installed apps. From 630cccca51317ad4583feba0fb86156c406b18d8 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Mon, 7 Jun 2021 14:54:09 +0200 Subject: [PATCH 08/26] Add app link SDK check --- .../java/com/duckduckgo/app/browser/BrowserWebViewClient.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index e20710969a08..5978088a6cdb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -112,7 +112,7 @@ class BrowserWebViewClient( } is SpecialUrlDetector.UrlType.AppLink -> { Timber.i("Found app link for ${urlType.url}") - if (isRedirect && appLinkTriggered || !isForMainFrame) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N || isRedirect && appLinkTriggered || !isForMainFrame) { return false } launchExternalApp(urlType) From 603a96b47a21a49098e31e38034adbb733b71085 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Mon, 7 Jun 2021 16:04:26 +0200 Subject: [PATCH 09/26] Update app link dialog title --- .../main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- app/src/main/res/values/strings.xml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 98bc8cd87ee0..1f3bb9c8f3ce 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -924,7 +924,7 @@ class BrowserTabFragment : if (isShowing != true) { alertDialog = AlertDialog.Builder(context) - .setTitle(R.string.launchingExternalApp) + .setTitle(R.string.appLinkDialogTitle) .setMessage(getString(R.string.confirmOpenExternalApp)) .setPositiveButton(R.string.yes) { _, _ -> onClick() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c4c89c0fffaa..c3dfb3499e59 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -68,6 +68,7 @@ Open Links in Associated Apps Disable to prevent links from automatically opening in other installed apps. + Open in another app? DuckDuckGo From 3bb6d6af1643559941e26a9a7c8b416879293f6a Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Wed, 9 Jun 2021 14:14:09 +0200 Subject: [PATCH 10/26] Refactor app link handling out of BrowserWebViewClient --- .../app/browser/BrowserWebViewClientTest.kt | 53 +++++++++++++++++-- .../app/browser/BrowserTabFragment.kt | 4 +- .../app/browser/BrowserWebViewClient.kt | 23 +++----- .../app/browser/applinks/AppLinksHandler.kt | 49 +++++++++++++++++ .../app/browser/di/BrowserModule.kt | 7 ++- 5 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index be5410dd4338..1159b624449d 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -17,6 +17,7 @@ package com.duckduckgo.app.browser import android.content.Context +import android.content.Intent import android.net.Uri import android.os.Build import android.webkit.* @@ -25,6 +26,7 @@ import androidx.test.annotation.UiThreadTest import androidx.test.filters.SdkSuppress import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.app.CoroutineTestRule +import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore @@ -70,6 +72,7 @@ class BrowserWebViewClientTest { private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val emailInjector: EmailInjector = mock() private val webResourceRequest: WebResourceRequest = mock() + private val appLinksHandler: AppLinksHandler = AppLinksHandler() @UiThreadTest @Before @@ -90,7 +93,8 @@ class BrowserWebViewClientTest { thirdPartyCookieManager, TestCoroutineScope(), coroutinesTestRule.testDispatcherProvider, - emailInjector + emailInjector, + appLinksHandler ) testee.webViewClientListener = listener whenever(webResourceRequest.isForMainFrame).thenReturn(false) @@ -278,12 +282,12 @@ class BrowserWebViewClientTest { } @Test - fun whenAppLinkTriggeredAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { + fun whenAppLinkOpenedInBrowserAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) whenever(webResourceRequest.isRedirect).thenReturn(true) whenever(webResourceRequest.isForMainFrame).thenReturn(true) - testee.appLinkTriggered = true + testee.appLinksHandler.enterBrowserState() assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener, never()).appLinkClicked(any()) } @@ -295,11 +299,36 @@ class BrowserWebViewClientTest { whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(false) + testee.appLinksHandler.enterBrowserState() assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener, never()).appLinkClicked(any()) } } + @Test + fun whenNonHttpAppLinkDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + whenever(webResourceRequest.isRedirect).thenReturn(false) + whenever(webResourceRequest.isForMainFrame).thenReturn(false) + testee.appLinksHandler.enterBrowserState() + assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener).nonHttpAppLinkClicked(any()) + } + } + + @Test + fun whenNonHttpRedirectDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndDontLaunchExternalApp() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + whenever(webResourceRequest.isRedirect).thenReturn(true) + whenever(webResourceRequest.isForMainFrame).thenReturn(false) + testee.appLinksHandler.enterBrowserState() + assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener, never()).nonHttpAppLinkClicked(any()) + } + } + @Test fun whenAppLinkDetectedOnApiLessThan24ThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { @@ -309,6 +338,24 @@ class BrowserWebViewClientTest { } } + @Test + fun whenNonHttpAppLinkDetectedOnApiLessThan24ThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + assertTrue(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) + verify(listener).nonHttpAppLinkClicked(any()) + } + } + + @UiThreadTest + @Test + fun whenOnPageStartedCalledThenResetAppLinkState() { + appLinksHandler.enterBrowserState() + assertTrue(appLinksHandler.appLinkOpenedInBrowser) + testee.onPageStarted(webView, EXAMPLE_URL, null) + assertFalse(appLinksHandler.appLinkOpenedInBrowser) + } + private class TestWebView(context: Context) : WebView(context) companion object { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 1f3bb9c8f3ce..24da8c5312f7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -928,12 +928,12 @@ class BrowserTabFragment : .setMessage(getString(R.string.confirmOpenExternalApp)) .setPositiveButton(R.string.yes) { _, _ -> onClick() - webViewClient.appLinkTriggered = false + webViewClient.appLinksHandler.reset() } .setNegativeButton(R.string.no) { _, _ -> if (url != null) { webView?.loadUrl(url, headers) - webViewClient.appLinkTriggered = true + webViewClient.appLinksHandler.enterBrowserState() } else { showToast(R.string.unableToOpenLink) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 5978088a6cdb..f5024c25cc7f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -26,6 +26,7 @@ import androidx.annotation.RequiresApi import androidx.annotation.UiThread import androidx.annotation.WorkerThread import androidx.core.net.toUri +import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationState import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager @@ -59,12 +60,12 @@ class BrowserWebViewClient( private val thirdPartyCookieManager: ThirdPartyCookieManager, private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, - private val emailInjector: EmailInjector + private val emailInjector: EmailInjector, + val appLinksHandler: AppLinksHandler ) : WebViewClient() { var webViewClientListener: WebViewClientListener? = null private var lastPageStarted: String? = null - var appLinkTriggered = false /** * This is the new method of url overriding available from API 24 onwards @@ -81,7 +82,7 @@ class BrowserWebViewClient( @Suppress("OverridingDeprecatedMember") override fun shouldOverrideUrlLoading(view: WebView, urlString: String): Boolean { val url = Uri.parse(urlString) - return shouldOverride(view, url, isForMainFrame = true, isRedirect = true) + return shouldOverride(view, url, isForMainFrame = true, isRedirect = false) } /** @@ -112,19 +113,11 @@ class BrowserWebViewClient( } is SpecialUrlDetector.UrlType.AppLink -> { Timber.i("Found app link for ${urlType.url}") - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N || isRedirect && appLinkTriggered || !isForMainFrame) { - return false - } - launchExternalApp(urlType) - true + return appLinksHandler.handleAppLink(isRedirect, isForMainFrame) { launchExternalApp(urlType) } } is SpecialUrlDetector.UrlType.NonHttpAppLink -> { - Timber.i("Found intent type link for ${urlType.url}") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && isRedirect && appLinkTriggered) { - return true - } - launchExternalApp(urlType) - true + Timber.i("Found non-http app link for ${urlType.url}") + return appLinksHandler.handleNonHttpAppLink(isRedirect) { launchExternalApp(urlType) } } is SpecialUrlDetector.UrlType.Unknown -> { Timber.w("Unable to process link type for ${urlType.url}") @@ -181,7 +174,7 @@ class BrowserWebViewClient( emailInjector.resetInjectedJsFlag() globalPrivacyControl.injectDoNotSellToDom(webView) loginDetector.onEvent(WebNavigationEvent.OnPageStarted(webView)) - appLinkTriggered = false + appLinksHandler.reset() } catch (e: Throwable) { appCoroutineScope.launch { uncaughtExceptionRepository.recordUncaughtException(e, ON_PAGE_STARTED) diff --git a/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt b/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt new file mode 100644 index 000000000000..85b46cd7e39d --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2021 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.applinks + +import android.os.Build +import javax.inject.Inject + +class AppLinksHandler @Inject constructor() { + + var appLinkOpenedInBrowser = false + + fun handleAppLink(isRedirect: Boolean, isForMainFrame: Boolean, launchAppLink: () -> Unit): Boolean { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N || isRedirect && appLinkOpenedInBrowser || !isForMainFrame) { + return false + } + launchAppLink() + return true + } + + fun handleNonHttpAppLink(isRedirect: Boolean, launchAppLink: () -> Unit): Boolean { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && isRedirect && appLinkOpenedInBrowser) { + return true + } + launchAppLink() + return true + } + + fun enterBrowserState() { + appLinkOpenedInBrowser = true + } + + fun reset() { + appLinkOpenedInBrowser = false + } +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index abecd35bc749..c266d6bf44c9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -21,6 +21,7 @@ import android.content.Context import android.webkit.CookieManager import android.webkit.WebSettings import androidx.lifecycle.LifecycleObserver +import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector import com.duckduckgo.app.browser.addtohome.AddToHomeSystemCapabilityDetector @@ -106,7 +107,8 @@ class BrowserModule { thirdPartyCookieManager: ThirdPartyCookieManager, @AppCoroutineScope appCoroutineScope: CoroutineScope, dispatcherProvider: DispatcherProvider, - emailInjector: EmailInjector + emailInjector: EmailInjector, + appLinksHandler: AppLinksHandler ): BrowserWebViewClient { return BrowserWebViewClient( webViewHttpAuthStore, @@ -123,7 +125,8 @@ class BrowserModule { thirdPartyCookieManager, appCoroutineScope, dispatcherProvider, - emailInjector + emailInjector, + appLinksHandler ) } From c11a097e6c46b21b07e8df90da398432344771b4 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Thu, 10 Jun 2021 10:47:56 +0200 Subject: [PATCH 11/26] Move untranslated strings to string-untranslated.xml --- app/src/main/res/values/string-untranslated.xml | 5 +++++ app/src/main/res/values/strings.xml | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/res/values/string-untranslated.xml b/app/src/main/res/values/string-untranslated.xml index 9cdb49495e01..e280b604ee4b 100644 --- a/app/src/main/res/values/string-untranslated.xml +++ b/app/src/main/res/values/string-untranslated.xml @@ -47,4 +47,9 @@ Success! %s has been added to your home screen. + + Open Links in Associated Apps + Disable to prevent links from automatically opening in other installed apps. + Open in another app? + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c3dfb3499e59..14c05dfef1d9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -65,11 +65,6 @@ Would you like to leave DuckDuckGo to view this content? Unable to open this type of link - - Open Links in Associated Apps - Disable to prevent links from automatically opening in other installed apps. - Open in another app? - DuckDuckGo Tabs From cf27a02d3c4162a758950bac6333ecc5cd707411 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Thu, 10 Jun 2021 11:42:45 +0200 Subject: [PATCH 12/26] Remove toast from app links dialog --- .../app/browser/BrowserTabViewModelTest.kt | 2 +- .../app/browser/BrowserWebViewClientTest.kt | 14 +++++++------- .../duckduckgo/app/browser/BrowserTabFragment.kt | 12 ++++-------- .../duckduckgo/app/browser/SpecialUrlDetector.kt | 2 +- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 2c07f423cccc..0c7a394c7718 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -3203,7 +3203,7 @@ class BrowserTabViewModelTest { @Test fun whenAppLinkClickedThenAppLinkCommandSent() { - testee.appLinkClicked(SpecialUrlDetector.UrlType.AppLink()) + testee.appLinkClicked(SpecialUrlDetector.UrlType.AppLink(url = "https://example.com")) assertCommandIssued() } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 1159b624449d..497c8b28fea5 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -272,7 +272,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - val urlType = SpecialUrlDetector.UrlType.AppLink() + val urlType = SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL) whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(true) @@ -284,7 +284,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkOpenedInBrowserAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) whenever(webResourceRequest.isRedirect).thenReturn(true) whenever(webResourceRequest.isForMainFrame).thenReturn(true) testee.appLinksHandler.enterBrowserState() @@ -296,7 +296,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedAndIsNotForMainFrameThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(false) testee.appLinksHandler.enterBrowserState() @@ -308,7 +308,7 @@ class BrowserWebViewClientTest { @Test fun whenNonHttpAppLinkDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(false) testee.appLinksHandler.enterBrowserState() @@ -320,7 +320,7 @@ class BrowserWebViewClientTest { @Test fun whenNonHttpRedirectDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndDontLaunchExternalApp() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) whenever(webResourceRequest.isRedirect).thenReturn(true) whenever(webResourceRequest.isForMainFrame).thenReturn(false) testee.appLinksHandler.enterBrowserState() @@ -332,7 +332,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedOnApiLessThan24ThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink()) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) verify(listener, never()).appLinkClicked(any()) } @@ -341,7 +341,7 @@ class BrowserWebViewClientTest { @Test fun whenNonHttpAppLinkDetectedOnApiLessThan24ThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "")) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) assertTrue(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) verify(listener).nonHttpAppLinkClicked(any()) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 24da8c5312f7..3f434a119857 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -805,7 +805,7 @@ class BrowserTabFragment : private fun openAppLinkDialog( appIntent: Intent?, excludedComponents: List?, - url: String? = null, + url: String, headers: Map = emptyMap() ) { context?.let { @@ -919,7 +919,7 @@ class BrowserTabFragment : } } - private fun launchAppLinkDialog(context: Context, url: String?, headers: Map, onClick: () -> Unit) { + private fun launchAppLinkDialog(context: Context, url: String, headers: Map, onClick: () -> Unit) { val isShowing = alertDialog?.isShowing if (isShowing != true) { @@ -931,12 +931,8 @@ class BrowserTabFragment : webViewClient.appLinksHandler.reset() } .setNegativeButton(R.string.no) { _, _ -> - if (url != null) { - webView?.loadUrl(url, headers) - webViewClient.appLinksHandler.enterBrowserState() - } else { - showToast(R.string.unableToOpenLink) - } + webView?.loadUrl(url, headers) + webViewClient.appLinksHandler.enterBrowserState() } .show() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index e7a2c2be73e8..523b97be5084 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -37,7 +37,7 @@ interface SpecialUrlDetector { class Telephone(val telephoneNumber: String) : UrlType() class Email(val emailAddress: String) : UrlType() class Sms(val telephoneNumber: String) : UrlType() - class AppLink(val appIntent: Intent? = null, val excludedComponents: List? = null, val url: String? = null) : UrlType() + class AppLink(val appIntent: Intent? = null, val excludedComponents: List? = null, val url: String) : UrlType() class NonHttpAppLink(val url: String, val intent: Intent, val fallbackUrl: String?) : UrlType() class SearchQuery(val query: String) : UrlType() class Unknown(val url: String) : UrlType() From bf3c44763ccf97e9618fa578ae86d44689c66908 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Thu, 10 Jun 2021 14:34:29 +0200 Subject: [PATCH 13/26] Extract URI flag --- .../java/com/duckduckgo/app/browser/SpecialUrlDetector.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 523b97be5084..323aecd28469 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -100,7 +100,7 @@ class SpecialUrlDetectorImpl( @Throws(URISyntaxException::class) private fun queryActivities(uriString: String): MutableList { - val browsableIntent = Intent.parseUri(uriString, 0) + val browsableIntent = Intent.parseUri(uriString, URI_FLAG) browsableIntent.addCategory(Intent.CATEGORY_BROWSABLE) return context.packageManager.queryIntentActivities(browsableIntent, PackageManager.GET_RESOLVED_FILTER) } @@ -112,7 +112,7 @@ class SpecialUrlDetectorImpl( } @Throws(URISyntaxException::class) private fun buildNonBrowserIntent(nonBrowserActivity: ResolveInfo, uriString: String): Intent { - val intent = Intent.parseUri(uriString, 0) + val intent = Intent.parseUri(uriString, URI_FLAG) intent.component = ComponentName(nonBrowserActivity.activityInfo.packageName, nonBrowserActivity.activityInfo.name) return intent } @@ -134,7 +134,7 @@ class SpecialUrlDetectorImpl( private fun buildIntent(uriString: String): UrlType { return try { - val intent = Intent.parseUri(uriString, 0) + val intent = Intent.parseUri(uriString, URI_FLAG) val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL) UrlType.NonHttpAppLink(url = uriString, intent = intent, fallbackUrl = fallbackUrl) } catch (e: URISyntaxException) { @@ -163,6 +163,7 @@ class SpecialUrlDetectorImpl( private const val DATA_SCHEME = "data" private const val JAVASCRIPT_SCHEME = "javascript" private const val EXTRA_FALLBACK_URL = "browser_fallback_url" + private const val URI_FLAG = 0 const val SMS_MAX_LENGTH = 400 const val PHONE_MAX_LENGTH = 20 const val EMAIL_MAX_LENGTH = 1000 From f3ddea7509c620c4813def5a2703437c2cb5f040 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Thu, 10 Jun 2021 14:57:03 +0200 Subject: [PATCH 14/26] Split up tests for different SDK versions --- .../app/browser/SpecialUrlDetectorImplTest.kt | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index 905d6d55721c..0f8089f6edfe 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -114,11 +114,10 @@ class SpecialUrlDetectorImplTest { @Test fun whenAppLinksEnabledAndOneNonBrowserActivityFoundThenReturnAppLinkWithIntent() { - whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) - whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) - val type = testee.determineType("https://example.com") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) + val type = testee.determineType("https://example.com") verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) assertTrue(type is AppLink) val appLinkType = type as AppLink @@ -126,19 +125,15 @@ class SpecialUrlDetectorImplTest { assertEquals(EXAMPLE_APP_PACKAGE, appLinkType.appIntent!!.component!!.packageName) assertEquals(EXAMPLE_APP_ACTIVITY_NAME, appLinkType.appIntent!!.component!!.className) assertNull(appLinkType.excludedComponents) - } else { - verifyZeroInteractions(mockPackageManager) - assertTrue(type is Web) } } @Test fun whenAppLinksEnabledAndMultipleNonBrowserActivitiesFoundThenReturnAppLinkWithExcludedComponents() { - whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) - whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) - val type = testee.determineType("https://example.com") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(listOf(buildAppResolveInfo(), buildAppResolveInfo(), buildBrowserResolveInfo(), ResolveInfo())) + val type = testee.determineType("https://example.com") verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) assertTrue(type is AppLink) val appLinkType = type as AppLink @@ -147,7 +142,14 @@ class SpecialUrlDetectorImplTest { assertEquals(EXAMPLE_BROWSER_PACKAGE, appLinkType.excludedComponents!![0].packageName) assertEquals(EXAMPLE_BROWSER_ACTIVITY_NAME, appLinkType.excludedComponents!![0].className) assertNull(appLinkType.appIntent) - } else { + } + } + + @Test + fun whenAppLinkCheckedOnApiLessThan24ThenReturnWebType() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) + val type = testee.determineType("https://example.com") verifyZeroInteractions(mockPackageManager) assertTrue(type is Web) } From df479aeec3565f8fc7f142624dbc576bbeda3df5 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Fri, 11 Jun 2021 13:26:31 +0200 Subject: [PATCH 15/26] Move app link handling to ViewModel --- .../app/browser/BrowserTabViewModelTest.kt | 49 ++++-- .../app/browser/BrowserWebViewClientTest.kt | 92 ++++++----- .../applinks/DuckDuckGoAppLinksHandlerTest.kt | 151 ++++++++++++++++++ .../app/browser/BrowserTabFragment.kt | 4 +- .../app/browser/BrowserTabViewModel.kt | 30 +++- .../app/browser/BrowserWebViewClient.kt | 24 ++- .../app/browser/WebViewClientListener.kt | 5 +- .../app/browser/applinks/AppLinksHandler.kt | 19 ++- .../app/browser/di/BrowserModule.kt | 7 +- 9 files changed, 301 insertions(+), 80 deletions(-) create mode 100644 app/src/androidTest/java/com/duckduckgo/app/browser/applinks/DuckDuckGoAppLinksHandlerTest.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 0c7a394c7718..4943f5393916 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -17,6 +17,7 @@ package com.duckduckgo.app.browser import android.content.Context +import android.content.Intent import android.graphics.Bitmap import android.net.Uri import android.view.MenuItem @@ -48,6 +49,7 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.FireButton import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector +import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.favicon.FaviconManager import com.duckduckgo.app.browser.favicon.FaviconSource @@ -113,15 +115,11 @@ import com.duckduckgo.app.trackerdetection.EntityLookup import com.duckduckgo.app.trackerdetection.model.TrackingEvent import com.duckduckgo.app.usage.search.SearchCountDao import com.duckduckgo.app.widget.ui.WidgetCapabilities +import com.nhaarman.mockitokotlin2.* import com.nhaarman.mockitokotlin2.any -import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.atLeastOnce -import com.nhaarman.mockitokotlin2.doAnswer import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.eq -import com.nhaarman.mockitokotlin2.firstValue -import com.nhaarman.mockitokotlin2.lastValue -import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import dagger.Lazy import io.reactivex.Observable @@ -149,6 +147,9 @@ import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito import org.mockito.Mockito.* +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations import org.mockito.internal.util.DefaultMockingDetails import java.io.File @@ -268,6 +269,9 @@ class BrowserTabViewModelTest { @Mock private lateinit var mockContext: Context + @Mock + private lateinit var mockAppLinksHandler: AppLinksHandler + private val lazyFaviconManager = Lazy { mockFaviconManager } private lateinit var mockAutoCompleteApi: AutoCompleteApi @@ -277,6 +281,9 @@ class BrowserTabViewModelTest { @Captor private lateinit var commandCaptor: ArgumentCaptor + @Captor + private lateinit var appLinkCaptor: ArgumentCaptor<() -> Unit> + private lateinit var db: AppDatabase private lateinit var testee: BrowserTabViewModel @@ -379,7 +386,8 @@ class BrowserTabViewModelTest { fireproofDialogsEventHandler = fireproofDialogsEventHandler, emailManager = mockEmailManager, favoritesRepository = mockFavoritesRepository, - appCoroutineScope = TestCoroutineScope() + appCoroutineScope = TestCoroutineScope(), + appLinksHandler = mockAppLinksHandler ) testee.loadData("abc", null, false) @@ -3202,12 +3210,35 @@ class BrowserTabViewModelTest { } @Test - fun whenAppLinkClickedThenAppLinkCommandSent() { - testee.appLinkClicked(SpecialUrlDetector.UrlType.AppLink(url = "https://example.com")) - + fun whenHandleAppLinkCalledThenHandleAppLink() { + val urlType = SpecialUrlDetector.UrlType.AppLink(url = "") + testee.handleAppLink(urlType, isRedirect = false, isForMainFrame = true) + verify(mockAppLinksHandler).handleAppLink(isRedirect = eq(false), isForMainFrame = eq(true), capture(appLinkCaptor)) + appLinkCaptor.value.invoke() assertCommandIssued() } + @Test + fun whenHandleNonHttpAppLinkCalledThenHandleNonHttpAppLink() { + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "") + testee.handleNonHttpAppLink(urlType, false) + verify(mockAppLinksHandler).handleNonHttpAppLink(isRedirect = eq(false), capture(appLinkCaptor)) + appLinkCaptor.value.invoke() + assertCommandIssued() + } + + @Test + fun whenResetAppLinkStateCalledThenResetAppLinkState() { + testee.resetAppLinkState() + verify(mockAppLinksHandler).reset() + } + + @Test + fun whenEnterAppLinkBrowserStateCalledThenEnterBrowserState() { + testee.enterAppLinkBrowserState() + verify(mockAppLinksHandler).enterBrowserState() + } + private suspend fun givenFireButtonPulsing() { whenever(mockUserStageStore.getUserAppStage()).thenReturn(AppStage.DAX_ONBOARDING) dismissedCtaDaoChannel.send(listOf(DismissedCta(CtaId.DAX_DIALOG_TRACKERS_FOUND))) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 497c8b28fea5..30805c69b5ca 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -26,7 +26,6 @@ import androidx.test.annotation.UiThreadTest import androidx.test.filters.SdkSuppress import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.app.CoroutineTestRule -import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore @@ -72,7 +71,6 @@ class BrowserWebViewClientTest { private val thirdPartyCookieManager: ThirdPartyCookieManager = mock() private val emailInjector: EmailInjector = mock() private val webResourceRequest: WebResourceRequest = mock() - private val appLinksHandler: AppLinksHandler = AppLinksHandler() @UiThreadTest @Before @@ -93,11 +91,9 @@ class BrowserWebViewClientTest { thirdPartyCookieManager, TestCoroutineScope(), coroutinesTestRule.testDispatcherProvider, - emailInjector, - appLinksHandler + emailInjector ) testee.webViewClientListener = listener - whenever(webResourceRequest.isForMainFrame).thenReturn(false) whenever(webResourceRequest.url).thenReturn(Uri.EMPTY) } @@ -270,90 +266,112 @@ class BrowserWebViewClientTest { } @Test - fun whenAppLinkDetectedThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + fun whenAppLinkDetectedAndIsHandledThenReturnTrue() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { val urlType = SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL) whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(true) + whenever(listener.handleAppLink(any(), any(), any())).thenReturn(true) assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(listener).appLinkClicked(urlType) + verify(listener).handleAppLink(urlType, isRedirect = false, isForMainFrame = true) } } @Test - fun whenAppLinkOpenedInBrowserAndIsRedirectThenReturnFalse() = coroutinesTestRule.runBlocking { + fun whenAppLinkDetectedAndIsNotHandledThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) - whenever(webResourceRequest.isRedirect).thenReturn(true) + val urlType = SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL) + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) + whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(true) - testee.appLinksHandler.enterBrowserState() + whenever(listener.handleAppLink(any(), any(), any())).thenReturn(false) assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(listener, never()).appLinkClicked(any()) + verify(listener).handleAppLink(urlType, isRedirect = false, isForMainFrame = true) } } @Test - fun whenAppLinkDetectedAndIsNotForMainFrameThenReturnFalse() = coroutinesTestRule.runBlocking { + fun whenAppLinkDetectedAndListenerIsNullThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) - whenever(webResourceRequest.isRedirect).thenReturn(false) - whenever(webResourceRequest.isForMainFrame).thenReturn(false) - testee.appLinksHandler.enterBrowserState() + testee.webViewClientListener = null assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(listener, never()).appLinkClicked(any()) + verify(listener, never()).handleAppLink(any(), any(), any()) } } @Test - fun whenNonHttpAppLinkDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + fun whenNonHttpAppLinkDetectedAndIsHandledThenReturnTrue() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL) + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) - whenever(webResourceRequest.isForMainFrame).thenReturn(false) - testee.appLinksHandler.enterBrowserState() + whenever(listener.handleNonHttpAppLink(any(), any())).thenReturn(true) assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(listener).nonHttpAppLinkClicked(any()) + verify(listener).handleNonHttpAppLink(urlType, isRedirect = false) + } + } + + @Test + fun whenNonHttpAppLinkDetectedAndIsHandledOnApiLessThan24ThenReturnTrue() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL) + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) + whenever(listener.handleNonHttpAppLink(any(), any())).thenReturn(true) + assertTrue(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) + verify(listener).handleNonHttpAppLink(urlType, isRedirect = false) } } @Test - fun whenNonHttpRedirectDetectedAfterAppLinkOpenedInBrowserThenReturnTrueAndDontLaunchExternalApp() = coroutinesTestRule.runBlocking { + fun whenNonHttpAppLinkDetectedAndIsNotHandledThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) - whenever(webResourceRequest.isRedirect).thenReturn(true) - whenever(webResourceRequest.isForMainFrame).thenReturn(false) - testee.appLinksHandler.enterBrowserState() - assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) - verify(listener, never()).nonHttpAppLinkClicked(any()) + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL) + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) + whenever(webResourceRequest.isRedirect).thenReturn(false) + whenever(listener.handleNonHttpAppLink(any(), any())).thenReturn(false) + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener).handleNonHttpAppLink(urlType, isRedirect = false) } } @Test - fun whenAppLinkDetectedOnApiLessThan24ThenReturnFalse() = coroutinesTestRule.runBlocking { + fun whenNonHttpAppLinkDetectedAndIsNotHandledOnApiLessThan24ThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL) + whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) + whenever(listener.handleNonHttpAppLink(any(), any())).thenReturn(false) assertFalse(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) - verify(listener, never()).appLinkClicked(any()) + verify(listener).handleNonHttpAppLink(urlType, isRedirect = false) + } + } + + @Test + fun whenNonHttpAppLinkDetectedAndListenerIsNullThenReturnTrue() = coroutinesTestRule.runBlocking { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) + testee.webViewClientListener = null + assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener, never()).handleNonHttpAppLink(any(), any()) } } @Test - fun whenNonHttpAppLinkDetectedOnApiLessThan24ThenReturnTrueAndLaunchExternalApp() = coroutinesTestRule.runBlocking { + fun whenNonHttpAppLinkDetectedAndListenerIsNullOnApiLessThan24ThenReturnTrue() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.NonHttpAppLink(EXAMPLE_URL, Intent(), EXAMPLE_URL)) + testee.webViewClientListener = null assertTrue(testee.shouldOverrideUrlLoading(webView, EXAMPLE_URL)) - verify(listener).nonHttpAppLinkClicked(any()) + verify(listener, never()).handleNonHttpAppLink(any(), any()) } } @UiThreadTest @Test fun whenOnPageStartedCalledThenResetAppLinkState() { - appLinksHandler.enterBrowserState() - assertTrue(appLinksHandler.appLinkOpenedInBrowser) testee.onPageStarted(webView, EXAMPLE_URL, null) - assertFalse(appLinksHandler.appLinkOpenedInBrowser) + verify(listener).resetAppLinkState() } private class TestWebView(context: Context) : WebView(context) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/applinks/DuckDuckGoAppLinksHandlerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/applinks/DuckDuckGoAppLinksHandlerTest.kt new file mode 100644 index 000000000000..a480cbeacbb1 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/applinks/DuckDuckGoAppLinksHandlerTest.kt @@ -0,0 +1,151 @@ +/* + * Copyright (c) 2021 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.applinks + +import android.os.Build +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyZeroInteractions +import junit.framework.TestCase.assertFalse +import junit.framework.TestCase.assertTrue +import org.junit.Before +import org.junit.Test + +class DuckDuckGoAppLinksHandlerTest { + + private lateinit var testee: DuckDuckGoAppLinksHandler + + private var mockCallback: () -> Unit = mock() + + @Before + fun setup() { + testee = DuckDuckGoAppLinksHandler() + } + + @Test + fun whenAppLinkHandledAndIsRedirectAndAppLinkNotOpenedInBrowserThenReturnTrueAndLaunchAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = false + assertTrue(testee.handleAppLink(isRedirect = true, isForMainFrame = true, launchAppLink = mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenAppLinkHandledAndIsNotRedirectAndAppLinkOpenedInBrowserThenReturnTrueAndLaunchAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertTrue(testee.handleAppLink(isRedirect = false, isForMainFrame = true, launchAppLink = mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenAppLinkHandledAndIsRedirectAndAppLinkOpenedInBrowserThenReturnFalse() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertFalse(testee.handleAppLink(isRedirect = true, isForMainFrame = true, launchAppLink = mockCallback)) + verifyZeroInteractions(mockCallback) + } + } + + @Test + fun whenAppLinkHandledAndIsForMainFrameThenReturnTrueAndLaunchAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = false + assertTrue(testee.handleAppLink(isRedirect = false, isForMainFrame = true, launchAppLink = mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenAppLinkHandledAndIsNotForMainFrameThenReturnFalse() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = false + assertFalse(testee.handleAppLink(isRedirect = false, isForMainFrame = false, launchAppLink = mockCallback)) + verifyZeroInteractions(mockCallback) + } + } + + @Test + fun whenAppLinkHandledOnApiLessThan24ThenReturnFalse() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertFalse(testee.handleAppLink(isRedirect = true, isForMainFrame = false, launchAppLink = mockCallback)) + verifyZeroInteractions(mockCallback) + } + } + + @Test + fun whenNonHttpAppLinkHandledAndIsRedirectAndAppLinkOpenedInBrowserThenReturnTrue() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertTrue(testee.handleNonHttpAppLink(true, mockCallback)) + verifyZeroInteractions(mockCallback) + } + } + + @Test + fun whenNonHttpAppLinkHandledAndIsRedirectAndAppLinkOpenedInBrowserThenReturnTrueAndLaunchNonHttpAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = false + assertTrue(testee.handleNonHttpAppLink(true, mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenNonHttpAppLinkHandledAndIsNotRedirectAndAppLinkOpenedInBrowserThenReturnTrueAndLaunchNonHttpAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertTrue(testee.handleNonHttpAppLink(false, mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenNonHttpAppLinkHandledAndIsNotRedirectAndAppLinkNotOpenedInBrowserThenReturnTrueAndLaunchNonHttpAppLink() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = false + assertTrue(testee.handleNonHttpAppLink(false, mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenNonHttpAppLinkHandledOnApiLessThan24ThenReturnTrueAndLaunchNonHttpAppLink() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + testee.appLinkOpenedInBrowser = true + assertTrue(testee.handleNonHttpAppLink(true, mockCallback)) + verify(mockCallback).invoke() + } + } + + @Test + fun whenEnterBrowserStateCalledThenSetAppLinkOpenedInBrowserToTrue() { + assertFalse(testee.appLinkOpenedInBrowser) + testee.enterBrowserState() + assertTrue(testee.appLinkOpenedInBrowser) + } + + @Test + fun whenResetCalledThenSetAppLinkOpenedInBrowserToFalse() { + testee.appLinkOpenedInBrowser = true + testee.reset() + assertFalse(testee.appLinkOpenedInBrowser) + } +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 3f434a119857..e1c9731f8da4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -928,11 +928,11 @@ class BrowserTabFragment : .setMessage(getString(R.string.confirmOpenExternalApp)) .setPositiveButton(R.string.yes) { _, _ -> onClick() - webViewClient.appLinksHandler.reset() + viewModel.resetAppLinkState() } .setNegativeButton(R.string.no) { _, _ -> webView?.loadUrl(url, headers) - webViewClient.appLinksHandler.enterBrowserState() + viewModel.enterAppLinkBrowserState() } .show() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 1beddb7e9a7b..4d3c96b077f0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -53,6 +53,8 @@ import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.AppLink import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.NonHttpAppLink import com.duckduckgo.app.browser.WebNavigationStateChange.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector +import com.duckduckgo.app.browser.applinks.AppLinksHandler +import com.duckduckgo.app.browser.applinks.DuckDuckGoAppLinksHandler import com.duckduckgo.app.browser.downloader.DownloadFailReason import com.duckduckgo.app.browser.downloader.FileDownloader import com.duckduckgo.app.browser.favicon.FaviconManager @@ -156,7 +158,8 @@ class BrowserTabViewModel( private val globalPrivacyControl: GlobalPrivacyControl, private val fireproofDialogsEventHandler: FireproofDialogsEventHandler, private val emailManager: EmailManager, - @AppCoroutineScope private val appCoroutineScope: CoroutineScope + @AppCoroutineScope private val appCoroutineScope: CoroutineScope, + private val appLinksHandler: AppLinksHandler ) : WebViewClientListener, EditSavedSiteListener, HttpAuthenticationListener, SiteLocationPermissionDialog.SiteLocationPermissionDialogListener, SystemLocationPermissionDialog.SystemLocationPermissionDialogListener, ViewModel() { @@ -1754,11 +1757,27 @@ class BrowserTabViewModel( tabRepository.updateTabPreviewImage(tabId, null) } - override fun appLinkClicked(appLink: AppLink) { + override fun handleAppLink(appLink: AppLink, isRedirect: Boolean, isForMainFrame: Boolean): Boolean { + return appLinksHandler.handleAppLink(isRedirect, isForMainFrame) { appLinkClicked(appLink) } + } + + override fun resetAppLinkState() { + appLinksHandler.reset() + } + + fun enterAppLinkBrowserState() { + appLinksHandler.enterBrowserState() + } + + fun appLinkClicked(appLink: AppLink) { command.value = HandleAppLink(appLink, getUrlHeaders()) } - override fun nonHttpAppLinkClicked(appLink: NonHttpAppLink) { + override fun handleNonHttpAppLink(nonHttpAppLink: NonHttpAppLink, isRedirect: Boolean): Boolean { + return appLinksHandler.handleNonHttpAppLink(isRedirect) { nonHttpAppLinkClicked(nonHttpAppLink) } + } + + fun nonHttpAppLinkClicked(appLink: NonHttpAppLink) { command.value = HandleNonHttpAppLink(appLink, getUrlHeaders()) } @@ -1992,12 +2011,13 @@ class BrowserTabViewModelFactory @Inject constructor( private val globalPrivacyControl: Provider, private val fireproofDialogsEventHandler: Provider, private val emailManager: Provider, - private val appCoroutineScope: Provider + private val appCoroutineScope: Provider, + private val appLinksHandler: Provider ) : ViewModelFactoryPlugin { override fun create(modelClass: Class): T? { with(modelClass) { return when { - isAssignableFrom(BrowserTabViewModel::class.java) -> BrowserTabViewModel(statisticsUpdater.get(), queryUrlConverter.get(), duckDuckGoUrlDetector.get(), siteFactory.get(), tabRepository.get(), userWhitelistDao.get(), networkLeaderboardDao.get(), bookmarksDao.get(), favoritesRepository.get(), fireproofWebsiteRepository.get(), locationPermissionsRepository.get(), geoLocationPermissions.get(), navigationAwareLoginDetector.get(), autoComplete.get(), appSettingsPreferencesStore.get(), longPressHandler.get(), webViewSessionStorage.get(), specialUrlDetector.get(), faviconManager.get(), addToHomeCapabilityDetector.get(), ctaViewModel.get(), searchCountDao.get(), pixel.get(), dispatchers, userEventsStore.get(), notificationDao.get(), variantManager.get(), fileDownloader.get(), globalPrivacyControl.get(), fireproofDialogsEventHandler.get(), emailManager.get(), appCoroutineScope.get()) as T + isAssignableFrom(BrowserTabViewModel::class.java) -> BrowserTabViewModel(statisticsUpdater.get(), queryUrlConverter.get(), duckDuckGoUrlDetector.get(), siteFactory.get(), tabRepository.get(), userWhitelistDao.get(), networkLeaderboardDao.get(), bookmarksDao.get(), favoritesRepository.get(), fireproofWebsiteRepository.get(), locationPermissionsRepository.get(), geoLocationPermissions.get(), navigationAwareLoginDetector.get(), autoComplete.get(), appSettingsPreferencesStore.get(), longPressHandler.get(), webViewSessionStorage.get(), specialUrlDetector.get(), faviconManager.get(), addToHomeCapabilityDetector.get(), ctaViewModel.get(), searchCountDao.get(), pixel.get(), dispatchers, userEventsStore.get(), notificationDao.get(), variantManager.get(), fileDownloader.get(), globalPrivacyControl.get(), fireproofDialogsEventHandler.get(), emailManager.get(), appCoroutineScope.get(), appLinksHandler.get()) as T else -> null } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index f5024c25cc7f..e738c6a76351 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -26,7 +26,6 @@ import androidx.annotation.RequiresApi import androidx.annotation.UiThread import androidx.annotation.WorkerThread import androidx.core.net.toUri -import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationState import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager @@ -60,8 +59,7 @@ class BrowserWebViewClient( private val thirdPartyCookieManager: ThirdPartyCookieManager, private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, - private val emailInjector: EmailInjector, - val appLinksHandler: AppLinksHandler + private val emailInjector: EmailInjector ) : WebViewClient() { var webViewClientListener: WebViewClientListener? = null @@ -113,11 +111,17 @@ class BrowserWebViewClient( } is SpecialUrlDetector.UrlType.AppLink -> { Timber.i("Found app link for ${urlType.url}") - return appLinksHandler.handleAppLink(isRedirect, isForMainFrame) { launchExternalApp(urlType) } + webViewClientListener?.let { listener -> + return listener.handleAppLink(urlType, isRedirect, isForMainFrame) + } + false } is SpecialUrlDetector.UrlType.NonHttpAppLink -> { Timber.i("Found non-http app link for ${urlType.url}") - return appLinksHandler.handleNonHttpAppLink(isRedirect) { launchExternalApp(urlType) } + webViewClientListener?.let { listener -> + return listener.handleNonHttpAppLink(urlType, isRedirect) + } + true } is SpecialUrlDetector.UrlType.Unknown -> { Timber.w("Unable to process link type for ${urlType.url}") @@ -148,14 +152,6 @@ class BrowserWebViewClient( } } - private fun launchExternalApp(urlType: SpecialUrlDetector.UrlType.NonHttpAppLink) { - webViewClientListener?.nonHttpAppLinkClicked(urlType) - } - - private fun launchExternalApp(urlType: SpecialUrlDetector.UrlType.AppLink) { - webViewClientListener?.appLinkClicked(urlType) - } - @UiThread override fun onPageStarted(webView: WebView, url: String?, favicon: Bitmap?) { try { @@ -174,7 +170,7 @@ class BrowserWebViewClient( emailInjector.resetInjectedJsFlag() globalPrivacyControl.injectDoNotSellToDom(webView) loginDetector.onEvent(WebNavigationEvent.OnPageStarted(webView)) - appLinksHandler.reset() + webViewClientListener?.resetAppLinkState() } catch (e: Throwable) { appCoroutineScope.launch { uncaughtExceptionRepository.recordUncaughtException(e, ON_PAGE_STARTED) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt index cb956c8a7974..4ebad0599c26 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt @@ -47,8 +47,9 @@ interface WebViewClientListener { fun goFullScreen(view: View) fun exitFullScreen() fun showFileChooser(filePathCallback: ValueCallback>, fileChooserParams: WebChromeClient.FileChooserParams) - fun nonHttpAppLinkClicked(appLink: SpecialUrlDetector.UrlType.NonHttpAppLink) - fun appLinkClicked(appLink: SpecialUrlDetector.UrlType.AppLink) + fun handleAppLink(appLink: SpecialUrlDetector.UrlType.AppLink, isRedirect: Boolean, isForMainFrame: Boolean): Boolean + fun resetAppLinkState() + fun handleNonHttpAppLink(nonHttpAppLink: SpecialUrlDetector.UrlType.NonHttpAppLink, isRedirect: Boolean): Boolean fun openMessageInNewTab(message: Message) fun recoverFromRenderProcessGone() fun requiresAuthentication(request: BasicAuthenticationRequest) diff --git a/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt b/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt index 85b46cd7e39d..1bbaebe89679 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/applinks/AppLinksHandler.kt @@ -19,11 +19,18 @@ package com.duckduckgo.app.browser.applinks import android.os.Build import javax.inject.Inject -class AppLinksHandler @Inject constructor() { +interface AppLinksHandler { + fun handleAppLink(isRedirect: Boolean, isForMainFrame: Boolean, launchAppLink: () -> Unit): Boolean + fun handleNonHttpAppLink(isRedirect: Boolean, launchNonHttpAppLink: () -> Unit): Boolean + fun enterBrowserState() + fun reset() +} + +class DuckDuckGoAppLinksHandler @Inject constructor() : AppLinksHandler { var appLinkOpenedInBrowser = false - fun handleAppLink(isRedirect: Boolean, isForMainFrame: Boolean, launchAppLink: () -> Unit): Boolean { + override fun handleAppLink(isRedirect: Boolean, isForMainFrame: Boolean, launchAppLink: () -> Unit): Boolean { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N || isRedirect && appLinkOpenedInBrowser || !isForMainFrame) { return false } @@ -31,19 +38,19 @@ class AppLinksHandler @Inject constructor() { return true } - fun handleNonHttpAppLink(isRedirect: Boolean, launchAppLink: () -> Unit): Boolean { + override fun handleNonHttpAppLink(isRedirect: Boolean, launchNonHttpAppLink: () -> Unit): Boolean { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && isRedirect && appLinkOpenedInBrowser) { return true } - launchAppLink() + launchNonHttpAppLink() return true } - fun enterBrowserState() { + override fun enterBrowserState() { appLinkOpenedInBrowser = true } - fun reset() { + override fun reset() { appLinkOpenedInBrowser = false } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index c266d6bf44c9..abecd35bc749 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -21,7 +21,6 @@ import android.content.Context import android.webkit.CookieManager import android.webkit.WebSettings import androidx.lifecycle.LifecycleObserver -import com.duckduckgo.app.browser.applinks.AppLinksHandler import com.duckduckgo.app.browser.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector import com.duckduckgo.app.browser.addtohome.AddToHomeSystemCapabilityDetector @@ -107,8 +106,7 @@ class BrowserModule { thirdPartyCookieManager: ThirdPartyCookieManager, @AppCoroutineScope appCoroutineScope: CoroutineScope, dispatcherProvider: DispatcherProvider, - emailInjector: EmailInjector, - appLinksHandler: AppLinksHandler + emailInjector: EmailInjector ): BrowserWebViewClient { return BrowserWebViewClient( webViewHttpAuthStore, @@ -125,8 +123,7 @@ class BrowserModule { thirdPartyCookieManager, appCoroutineScope, dispatcherProvider, - emailInjector, - appLinksHandler + emailInjector ) } From ee8efe3e5b660831454bdd2dc25856783aced04f Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Fri, 11 Jun 2021 13:32:34 +0200 Subject: [PATCH 16/26] Extract constant for URI flag --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index e1c9731f8da4..819ae3d666db 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -821,7 +821,7 @@ class BrowserTabFragment : @RequiresApi(Build.VERSION_CODES.N) private fun getChooserIntent(url: String?, title: String, excludedComponents: List): Intent { - val urlIntent = Intent.parseUri(url, 0) + val urlIntent = Intent.parseUri(url, URI_FLAG) val chooserIntent = Intent.createChooser(urlIntent, title) chooserIntent.putExtra(Intent.EXTRA_EXCLUDE_COMPONENTS, excludedComponents.toTypedArray()) return chooserIntent @@ -1631,6 +1631,8 @@ class BrowserTabFragment : private const val QUICK_ACCESS_GRID_MAX_COLUMNS = 6 + private const val URI_FLAG = 0 + fun newInstance(tabId: String, query: String? = null, skipHome: Boolean): BrowserTabFragment { val fragment = BrowserTabFragment() val args = Bundle() From b4e6f1d295d628097745325762cfc13fb1983ea7 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Fri, 11 Jun 2021 14:01:24 +0200 Subject: [PATCH 17/26] Inject PackageManager instead of Context --- .../com/duckduckgo/app/browser/BrowserTabViewModelTest.kt | 6 +++--- .../duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt | 7 +------ .../java/com/duckduckgo/app/browser/SpecialUrlDetector.kt | 5 ++--- .../java/com/duckduckgo/app/browser/di/BrowserModule.kt | 3 ++- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 4943f5393916..cb156184e62b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -16,8 +16,8 @@ package com.duckduckgo.app.browser -import android.content.Context import android.content.Intent +import android.content.pm.PackageManager import android.graphics.Bitmap import android.net.Uri import android.view.MenuItem @@ -267,7 +267,7 @@ class BrowserTabViewModelTest { private lateinit var mockFavoritesRepository: FavoritesRepository @Mock - private lateinit var mockContext: Context + private lateinit var mockPackageManager: PackageManager @Mock private lateinit var mockAppLinksHandler: AppLinksHandler @@ -363,7 +363,7 @@ class BrowserTabViewModelTest { bookmarksDao = mockBookmarksDao, longPressHandler = mockLongPressHandler, webViewSessionStorage = webViewSessionStorage, - specialUrlDetector = SpecialUrlDetectorImpl(mockContext, mockSettingsStore), + specialUrlDetector = SpecialUrlDetectorImpl(mockPackageManager, mockSettingsStore), faviconManager = mockFaviconManager, addToHomeCapabilityDetector = mockAddToHomeCapabilityDetector, ctaViewModel = ctaViewModel, diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index 0f8089f6edfe..d336a2a24b55 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -16,7 +16,6 @@ package com.duckduckgo.app.browser -import android.content.Context import android.content.Intent import android.content.IntentFilter import android.content.pm.ActivityInfo @@ -43,9 +42,6 @@ class SpecialUrlDetectorImplTest { lateinit var testee: SpecialUrlDetector - @Mock - lateinit var mockContext: Context - @Mock lateinit var mockPackageManager: PackageManager @@ -55,8 +51,7 @@ class SpecialUrlDetectorImplTest { @Before fun setup() { MockitoAnnotations.openMocks(this) - testee = SpecialUrlDetectorImpl(mockContext, mockSettingsDataStore) - whenever(mockContext.packageManager).thenReturn(mockPackageManager) + testee = SpecialUrlDetectorImpl(mockPackageManager, mockSettingsDataStore) whenever(mockPackageManager.queryIntentActivities(any(), anyInt())).thenReturn(emptyList()) whenever(mockSettingsDataStore.appLinksEnabled).thenReturn(true) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 323aecd28469..348a6f6e93d0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -17,7 +17,6 @@ package com.duckduckgo.app.browser import android.content.ComponentName -import android.content.Context import android.content.Intent import android.content.pm.PackageManager import android.content.pm.ResolveInfo @@ -45,7 +44,7 @@ interface SpecialUrlDetector { } class SpecialUrlDetectorImpl( - private val context: Context, + private val packageManager: PackageManager, private val settingsDataStore: SettingsDataStore ) : SpecialUrlDetector { @@ -102,7 +101,7 @@ class SpecialUrlDetectorImpl( private fun queryActivities(uriString: String): MutableList { val browsableIntent = Intent.parseUri(uriString, URI_FLAG) browsableIntent.addCategory(Intent.CATEGORY_BROWSABLE) - return context.packageManager.queryIntentActivities(browsableIntent, PackageManager.GET_RESOLVED_FILTER) + return packageManager.queryIntentActivities(browsableIntent, PackageManager.GET_RESOLVED_FILTER) } private fun keepNonBrowserActivities(activities: List): List { diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index abecd35bc749..08b578718146 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -18,6 +18,7 @@ package com.duckduckgo.app.browser.di import android.content.ClipboardManager import android.content.Context +import android.content.pm.PackageManager import android.webkit.CookieManager import android.webkit.WebSettings import androidx.lifecycle.LifecycleObserver @@ -174,7 +175,7 @@ class BrowserModule { } @Provides - fun specialUrlDetector(context: Context, settingsDataStore: SettingsDataStore): SpecialUrlDetector = SpecialUrlDetectorImpl(context, settingsDataStore) + fun specialUrlDetector(packageManager: PackageManager, settingsDataStore: SettingsDataStore): SpecialUrlDetector = SpecialUrlDetectorImpl(packageManager, settingsDataStore) @Provides @Singleton From 75f575ae699655d524f780585603177c5ea6bc69 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Fri, 11 Jun 2021 14:16:22 +0200 Subject: [PATCH 18/26] Remove context check when launching dialog --- .../duckduckgo/app/browser/BrowserTabFragment.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 819ae3d666db..f37de0dec27f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -808,14 +808,12 @@ class BrowserTabFragment : url: String, headers: Map = emptyMap() ) { - context?.let { - if (appIntent != null) { - launchAppLinkDialog(it, url, headers) { it.startActivity(appIntent) } - } else if (excludedComponents != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - val title = getString(R.string.openExternalApp) - val chooserIntent = getChooserIntent(url, title, excludedComponents) - launchAppLinkDialog(it, url, headers) { it.startActivity(chooserIntent) } - } + if (appIntent != null) { + launchAppLinkDialog(requireContext(), url, headers) { startActivity(appIntent) } + } else if (excludedComponents != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + val title = getString(R.string.openExternalApp) + val chooserIntent = getChooserIntent(url, title, excludedComponents) + launchAppLinkDialog(requireContext(), url, headers) { startActivity(chooserIntent) } } } From d572547aee683671b653521a8ad36638a0b140c3 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Fri, 11 Jun 2021 16:08:21 +0200 Subject: [PATCH 19/26] Open app link in browser when entered by user --- .../app/browser/BrowserTabViewModelTest.kt | 26 ++++++++++++++----- .../app/browser/BrowserTabFragment.kt | 15 ++++++++--- .../app/browser/BrowserTabViewModel.kt | 23 +++++++++++----- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index cb156184e62b..c7fc7ee1a24b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -17,7 +17,6 @@ package com.duckduckgo.app.browser import android.content.Intent -import android.content.pm.PackageManager import android.graphics.Bitmap import android.net.Uri import android.view.MenuItem @@ -142,7 +141,6 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.mockito.ArgumentCaptor -import org.mockito.ArgumentMatchers.anyString import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito @@ -267,7 +265,7 @@ class BrowserTabViewModelTest { private lateinit var mockFavoritesRepository: FavoritesRepository @Mock - private lateinit var mockPackageManager: PackageManager + private lateinit var mockSpecialUrlDetector: SpecialUrlDetector @Mock private lateinit var mockAppLinksHandler: AppLinksHandler @@ -363,7 +361,7 @@ class BrowserTabViewModelTest { bookmarksDao = mockBookmarksDao, longPressHandler = mockLongPressHandler, webViewSessionStorage = webViewSessionStorage, - specialUrlDetector = SpecialUrlDetectorImpl(mockPackageManager, mockSettingsStore), + specialUrlDetector = mockSpecialUrlDetector, faviconManager = mockFaviconManager, addToHomeCapabilityDetector = mockAddToHomeCapabilityDetector, ctaViewModel = ctaViewModel, @@ -1137,6 +1135,14 @@ class BrowserTabViewModelTest { assertTrue(commandCaptor.allValues.any { it == Command.HideKeyboard }) } + @Test + fun whenEnteringAppLinkQueryThenNavigateInBrowser() { + whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com") + testee.onUserSubmittedQuery("foo") + verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) + assertTrue(commandCaptor.allValues.any { it == Command.HideKeyboard }) + } + @Test fun whenNotifiedEnteringFullScreenThenViewStateUpdatedWithFullScreenFlag() { val stubView = View(getInstrumentation().targetContext) @@ -3211,7 +3217,7 @@ class BrowserTabViewModelTest { @Test fun whenHandleAppLinkCalledThenHandleAppLink() { - val urlType = SpecialUrlDetector.UrlType.AppLink(url = "") + val urlType = SpecialUrlDetector.UrlType.AppLink(url = "http://example.com") testee.handleAppLink(urlType, isRedirect = false, isForMainFrame = true) verify(mockAppLinksHandler).handleAppLink(isRedirect = eq(false), isForMainFrame = eq(true), capture(appLinkCaptor)) appLinkCaptor.value.invoke() @@ -3220,7 +3226,7 @@ class BrowserTabViewModelTest { @Test fun whenHandleNonHttpAppLinkCalledThenHandleNonHttpAppLink() { - val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("", Intent(), "") + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("http://example.com", Intent(), "http://example.com") testee.handleNonHttpAppLink(urlType, false) verify(mockAppLinksHandler).handleNonHttpAppLink(isRedirect = eq(false), capture(appLinkCaptor)) appLinkCaptor.value.invoke() @@ -3239,6 +3245,14 @@ class BrowserTabViewModelTest { verify(mockAppLinksHandler).enterBrowserState() } + @Test + fun whenUserSubmittedQueryIsAppLinkThenOpenAppLinkInBrowser() { + whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com") + whenever(mockSpecialUrlDetector.determineType(anyString())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = "http://foo.com")) + testee.onUserSubmittedQuery("foo") + assertCommandIssued() + } + private suspend fun givenFireButtonPulsing() { whenever(mockUserStageStore.getUserAppStage()).thenReturn(AppStage.DAX_ONBOARDING) dismissedCtaDaoChannel.send(listOf(DismissedCta(CtaId.DAX_DIALOG_TRACKERS_FOUND))) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index f37de0dec27f..44a05330933b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -630,6 +630,9 @@ class BrowserTabFragment : is Command.HandleAppLink -> { openAppLinkDialog(it.appLink.appIntent, it.appLink.excludedComponents, it.appLink.url, it.headers) } + is Command.OpenAppLinkInBrowser -> { + openAppLinkInBrowser(it.url, it.headers) + } is Command.HandleNonHttpAppLink -> { openExternalDialog(it.nonHttpAppLink.intent, it.nonHttpAppLink.fallbackUrl, false, it.headers) } @@ -917,7 +920,7 @@ class BrowserTabFragment : } } - private fun launchAppLinkDialog(context: Context, url: String, headers: Map, onClick: () -> Unit) { + private fun launchAppLinkDialog(context: Context, url: String, headers: Map, launchApp: () -> Unit) { val isShowing = alertDialog?.isShowing if (isShowing != true) { @@ -925,17 +928,21 @@ class BrowserTabFragment : .setTitle(R.string.appLinkDialogTitle) .setMessage(getString(R.string.confirmOpenExternalApp)) .setPositiveButton(R.string.yes) { _, _ -> - onClick() + launchApp() viewModel.resetAppLinkState() } .setNegativeButton(R.string.no) { _, _ -> - webView?.loadUrl(url, headers) - viewModel.enterAppLinkBrowserState() + openAppLinkInBrowser(url, headers) } .show() } } + private fun openAppLinkInBrowser(url: String, headers: Map) { + webView?.loadUrl(url, headers) + viewModel.enterAppLinkBrowserState() + } + override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { if (requestCode == REQUEST_CODE_CHOOSE_FILE) { handleFileUploadResult(resultCode, data) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 4d3c96b077f0..ff1d0bdb6a1f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -284,6 +284,7 @@ class BrowserTabViewModel( class ShowFileChooser(val filePathCallback: ValueCallback>, val fileChooserParams: WebChromeClient.FileChooserParams) : Command() class HandleNonHttpAppLink(val nonHttpAppLink: NonHttpAppLink, val headers: Map) : Command() class HandleAppLink(val appLink: AppLink, val headers: Map) : Command() + class OpenAppLinkInBrowser(val url: String, val headers: Map) : Command() class AddHomeShortcut(val title: String, val url: String, val icon: Bitmap? = null) : Command() class LaunchSurvey(val survey: Survey) : Command() object LaunchAddWidget : Command() @@ -573,15 +574,23 @@ class BrowserTabViewModel( val urlToNavigate = queryUrlConverter.convertQueryToUrl(trimmedInput, verticalParameter, queryOrigin) val type = specialUrlDetector.determineType(trimmedInput) - if (type is NonHttpAppLink) { - nonHttpAppLinkClicked(type) - } else { - if (shouldClearHistoryOnNewQuery()) { - command.value = ResetHistory + val urlType = specialUrlDetector.determineType(urlToNavigate) + + when { + type is NonHttpAppLink -> { + nonHttpAppLinkClicked(type) + } + urlType is AppLink -> { + command.value = OpenAppLinkInBrowser(urlToNavigate, getUrlHeaders()) } + else -> { + if (shouldClearHistoryOnNewQuery()) { + command.value = ResetHistory + } - fireQueryChangedPixel(trimmedInput) - command.value = Navigate(urlToNavigate, getUrlHeaders()) + fireQueryChangedPixel(trimmedInput) + command.value = Navigate(urlToNavigate, getUrlHeaders()) + } } globalLayoutState.value = Browser(isNewTabState = false) From 31fc3a30968aba1f622f3c6cd2beaf1a2384ae80 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Mon, 14 Jun 2021 08:30:03 +0200 Subject: [PATCH 20/26] Open app link redirects in browser when entered by user --- .../app/browser/BrowserTabViewModelTest.kt | 9 ++----- .../app/browser/BrowserTabFragment.kt | 11 ++------ .../app/browser/BrowserTabViewModel.kt | 27 +++++++------------ 3 files changed, 14 insertions(+), 33 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index c7fc7ee1a24b..35756258d932 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -3239,18 +3239,13 @@ class BrowserTabViewModelTest { verify(mockAppLinksHandler).reset() } - @Test - fun whenEnterAppLinkBrowserStateCalledThenEnterBrowserState() { - testee.enterAppLinkBrowserState() - verify(mockAppLinksHandler).enterBrowserState() - } - @Test fun whenUserSubmittedQueryIsAppLinkThenOpenAppLinkInBrowser() { whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com") whenever(mockSpecialUrlDetector.determineType(anyString())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = "http://foo.com")) testee.onUserSubmittedQuery("foo") - assertCommandIssued() + verify(mockAppLinksHandler).enterBrowserState() + assertCommandIssued() } private suspend fun givenFireButtonPulsing() { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 44a05330933b..c40df36a9146 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -630,9 +630,6 @@ class BrowserTabFragment : is Command.HandleAppLink -> { openAppLinkDialog(it.appLink.appIntent, it.appLink.excludedComponents, it.appLink.url, it.headers) } - is Command.OpenAppLinkInBrowser -> { - openAppLinkInBrowser(it.url, it.headers) - } is Command.HandleNonHttpAppLink -> { openExternalDialog(it.nonHttpAppLink.intent, it.nonHttpAppLink.fallbackUrl, false, it.headers) } @@ -932,17 +929,13 @@ class BrowserTabFragment : viewModel.resetAppLinkState() } .setNegativeButton(R.string.no) { _, _ -> - openAppLinkInBrowser(url, headers) + viewModel.openAppLinksInBrowser() + navigate(url, headers) } .show() } } - private fun openAppLinkInBrowser(url: String, headers: Map) { - webView?.loadUrl(url, headers) - viewModel.enterAppLinkBrowserState() - } - override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { if (requestCode == REQUEST_CODE_CHOOSE_FILE) { handleFileUploadResult(resultCode, data) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index ff1d0bdb6a1f..1f3ac9476514 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -284,7 +284,6 @@ class BrowserTabViewModel( class ShowFileChooser(val filePathCallback: ValueCallback>, val fileChooserParams: WebChromeClient.FileChooserParams) : Command() class HandleNonHttpAppLink(val nonHttpAppLink: NonHttpAppLink, val headers: Map) : Command() class HandleAppLink(val appLink: AppLink, val headers: Map) : Command() - class OpenAppLinkInBrowser(val url: String, val headers: Map) : Command() class AddHomeShortcut(val title: String, val url: String, val icon: Bitmap? = null) : Command() class LaunchSurvey(val survey: Survey) : Command() object LaunchAddWidget : Command() @@ -574,23 +573,17 @@ class BrowserTabViewModel( val urlToNavigate = queryUrlConverter.convertQueryToUrl(trimmedInput, verticalParameter, queryOrigin) val type = specialUrlDetector.determineType(trimmedInput) - val urlType = specialUrlDetector.determineType(urlToNavigate) - - when { - type is NonHttpAppLink -> { - nonHttpAppLinkClicked(type) - } - urlType is AppLink -> { - command.value = OpenAppLinkInBrowser(urlToNavigate, getUrlHeaders()) + if (type is NonHttpAppLink) { + nonHttpAppLinkClicked(type) + } else { + if (shouldClearHistoryOnNewQuery()) { + command.value = ResetHistory } - else -> { - if (shouldClearHistoryOnNewQuery()) { - command.value = ResetHistory - } - fireQueryChangedPixel(trimmedInput) - command.value = Navigate(urlToNavigate, getUrlHeaders()) - } + fireQueryChangedPixel(trimmedInput) + + openAppLinksInBrowser() + command.value = Navigate(urlToNavigate, getUrlHeaders()) } globalLayoutState.value = Browser(isNewTabState = false) @@ -1774,7 +1767,7 @@ class BrowserTabViewModel( appLinksHandler.reset() } - fun enterAppLinkBrowserState() { + fun openAppLinksInBrowser() { appLinksHandler.enterBrowserState() } From 15d81af7f5a1c20043d14671644b760e3d4f50e9 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 15:41:23 +0200 Subject: [PATCH 21/26] Rename URI_FLAG to URI_NO_FLAG --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- .../java/com/duckduckgo/app/browser/SpecialUrlDetector.kt | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index c40df36a9146..1594b5741959 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -819,7 +819,7 @@ class BrowserTabFragment : @RequiresApi(Build.VERSION_CODES.N) private fun getChooserIntent(url: String?, title: String, excludedComponents: List): Intent { - val urlIntent = Intent.parseUri(url, URI_FLAG) + val urlIntent = Intent.parseUri(url, URI_NO_FLAG) val chooserIntent = Intent.createChooser(urlIntent, title) chooserIntent.putExtra(Intent.EXTRA_EXCLUDE_COMPONENTS, excludedComponents.toTypedArray()) return chooserIntent @@ -1629,7 +1629,7 @@ class BrowserTabFragment : private const val QUICK_ACCESS_GRID_MAX_COLUMNS = 6 - private const val URI_FLAG = 0 + private const val URI_NO_FLAG = 0 fun newInstance(tabId: String, query: String? = null, skipHome: Boolean): BrowserTabFragment { val fragment = BrowserTabFragment() diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 348a6f6e93d0..4e186db888b5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -99,7 +99,7 @@ class SpecialUrlDetectorImpl( @Throws(URISyntaxException::class) private fun queryActivities(uriString: String): MutableList { - val browsableIntent = Intent.parseUri(uriString, URI_FLAG) + val browsableIntent = Intent.parseUri(uriString, URI_NO_FLAG) browsableIntent.addCategory(Intent.CATEGORY_BROWSABLE) return packageManager.queryIntentActivities(browsableIntent, PackageManager.GET_RESOLVED_FILTER) } @@ -111,7 +111,7 @@ class SpecialUrlDetectorImpl( } @Throws(URISyntaxException::class) private fun buildNonBrowserIntent(nonBrowserActivity: ResolveInfo, uriString: String): Intent { - val intent = Intent.parseUri(uriString, URI_FLAG) + val intent = Intent.parseUri(uriString, URI_NO_FLAG) intent.component = ComponentName(nonBrowserActivity.activityInfo.packageName, nonBrowserActivity.activityInfo.name) return intent } @@ -133,7 +133,7 @@ class SpecialUrlDetectorImpl( private fun buildIntent(uriString: String): UrlType { return try { - val intent = Intent.parseUri(uriString, URI_FLAG) + val intent = Intent.parseUri(uriString, URI_NO_FLAG) val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL) UrlType.NonHttpAppLink(url = uriString, intent = intent, fallbackUrl = fallbackUrl) } catch (e: URISyntaxException) { @@ -162,7 +162,7 @@ class SpecialUrlDetectorImpl( private const val DATA_SCHEME = "data" private const val JAVASCRIPT_SCHEME = "javascript" private const val EXTRA_FALLBACK_URL = "browser_fallback_url" - private const val URI_FLAG = 0 + private const val URI_NO_FLAG = 0 const val SMS_MAX_LENGTH = 400 const val PHONE_MAX_LENGTH = 20 const val EMAIL_MAX_LENGTH = 1000 From 5ba4a47302e8550f74a2d1bd05648c29e32fd424 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 15:53:31 +0200 Subject: [PATCH 22/26] Replace link in non http app link test --- .../java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 35756258d932..b6c3d90eccee 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -3226,7 +3226,7 @@ class BrowserTabViewModelTest { @Test fun whenHandleNonHttpAppLinkCalledThenHandleNonHttpAppLink() { - val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("http://example.com", Intent(), "http://example.com") + val urlType = SpecialUrlDetector.UrlType.NonHttpAppLink("market://details?id=com.example", Intent(), "http://example.com") testee.handleNonHttpAppLink(urlType, false) verify(mockAppLinksHandler).handleNonHttpAppLink(isRedirect = eq(false), capture(appLinkCaptor)) appLinkCaptor.value.invoke() From 2235a6a3ba46fd80b2911962d6fbd56640ebae32 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 18:30:42 +0200 Subject: [PATCH 23/26] Refactor browser filtering into separate method --- .../java/com/duckduckgo/app/browser/SpecialUrlDetector.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 4e186db888b5..a5562e3ba539 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -106,7 +106,7 @@ class SpecialUrlDetectorImpl( private fun keepNonBrowserActivities(activities: List): List { return activities.filter { resolveInfo -> - resolveInfo.filter != null && !(resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0) + resolveInfo.filter != null && !(isBrowserInfo(resolveInfo)) } } @Throws(URISyntaxException::class) @@ -118,10 +118,13 @@ class SpecialUrlDetectorImpl( private fun getExcludedComponents(activities: List): List { return activities.filter { resolveInfo -> - resolveInfo.filter != null && (resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0) + resolveInfo.filter != null && isBrowserInfo(resolveInfo) }.map { ComponentName(it.activityInfo.packageName, it.activityInfo.name) } } + private fun isBrowserInfo(resolveInfo: ResolveInfo) = + resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0 + private fun checkForIntent(scheme: String, uriString: String): UrlType { val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+") if (scheme.matches(validUriSchemeRegex)) { From 3fb25a8fcc8d5dc0bc5bee3ccb32e52a692248f2 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 18:36:01 +0200 Subject: [PATCH 24/26] Rename url to uriString --- .../app/browser/BrowserTabViewModelTest.kt | 4 ++-- .../app/browser/BrowserWebViewClientTest.kt | 6 +++--- .../app/browser/SpecialUrlDetectorImplTest.kt | 6 +++--- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- .../duckduckgo/app/browser/BrowserWebViewClient.kt | 6 +++--- .../com/duckduckgo/app/browser/SpecialUrlDetector.kt | 12 ++++++------ 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index b6c3d90eccee..291e9c21ff0d 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -3217,7 +3217,7 @@ class BrowserTabViewModelTest { @Test fun whenHandleAppLinkCalledThenHandleAppLink() { - val urlType = SpecialUrlDetector.UrlType.AppLink(url = "http://example.com") + val urlType = SpecialUrlDetector.UrlType.AppLink(uriString = "http://example.com") testee.handleAppLink(urlType, isRedirect = false, isForMainFrame = true) verify(mockAppLinksHandler).handleAppLink(isRedirect = eq(false), isForMainFrame = eq(true), capture(appLinkCaptor)) appLinkCaptor.value.invoke() @@ -3242,7 +3242,7 @@ class BrowserTabViewModelTest { @Test fun whenUserSubmittedQueryIsAppLinkThenOpenAppLinkInBrowser() { whenever(mockOmnibarConverter.convertQueryToUrl("foo", null)).thenReturn("foo.com") - whenever(mockSpecialUrlDetector.determineType(anyString())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = "http://foo.com")) + whenever(mockSpecialUrlDetector.determineType(anyString())).thenReturn(SpecialUrlDetector.UrlType.AppLink(uriString = "http://foo.com")) testee.onUserSubmittedQuery("foo") verify(mockAppLinksHandler).enterBrowserState() assertCommandIssued() diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 30805c69b5ca..7f25c7a9b23f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -268,7 +268,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedAndIsHandledThenReturnTrue() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - val urlType = SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL) + val urlType = SpecialUrlDetector.UrlType.AppLink(uriString = EXAMPLE_URL) whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(true) @@ -281,7 +281,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedAndIsNotHandledThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - val urlType = SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL) + val urlType = SpecialUrlDetector.UrlType.AppLink(uriString = EXAMPLE_URL) whenever(specialUrlDetector.determineType(any())).thenReturn(urlType) whenever(webResourceRequest.isRedirect).thenReturn(false) whenever(webResourceRequest.isForMainFrame).thenReturn(true) @@ -294,7 +294,7 @@ class BrowserWebViewClientTest { @Test fun whenAppLinkDetectedAndListenerIsNullThenReturnFalse() = coroutinesTestRule.runBlocking { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(url = EXAMPLE_URL)) + whenever(specialUrlDetector.determineType(any())).thenReturn(SpecialUrlDetector.UrlType.AppLink(uriString = EXAMPLE_URL)) testee.webViewClientListener = null assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener, never()).handleAppLink(any(), any(), any()) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt index d336a2a24b55..8cf486939549 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt @@ -116,7 +116,7 @@ class SpecialUrlDetectorImplTest { verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) assertTrue(type is AppLink) val appLinkType = type as AppLink - assertEquals("https://example.com", appLinkType.url) + assertEquals("https://example.com", appLinkType.uriString) assertEquals(EXAMPLE_APP_PACKAGE, appLinkType.appIntent!!.component!!.packageName) assertEquals(EXAMPLE_APP_ACTIVITY_NAME, appLinkType.appIntent!!.component!!.className) assertNull(appLinkType.excludedComponents) @@ -132,7 +132,7 @@ class SpecialUrlDetectorImplTest { verify(mockPackageManager).queryIntentActivities(argThat { hasCategory(Intent.CATEGORY_BROWSABLE) }, eq(PackageManager.GET_RESOLVED_FILTER)) assertTrue(type is AppLink) val appLinkType = type as AppLink - assertEquals("https://example.com", appLinkType.url) + assertEquals("https://example.com", appLinkType.uriString) assertEquals(1, appLinkType.excludedComponents!!.size) assertEquals(EXAMPLE_BROWSER_PACKAGE, appLinkType.excludedComponents!![0].packageName) assertEquals(EXAMPLE_BROWSER_ACTIVITY_NAME, appLinkType.excludedComponents!![0].className) @@ -225,7 +225,7 @@ class SpecialUrlDetectorImplTest { @Test fun whenUrlIsCustomUriSchemeThenNonHttpAppLinkTypeDetected() { val type = testee.determineType("myapp:foo bar") as NonHttpAppLink - assertEquals("myapp:foo bar", type.url) + assertEquals("myapp:foo bar", type.uriString) } @Test diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 1594b5741959..82ababed5579 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -628,7 +628,7 @@ class BrowserTabFragment : } } is Command.HandleAppLink -> { - openAppLinkDialog(it.appLink.appIntent, it.appLink.excludedComponents, it.appLink.url, it.headers) + openAppLinkDialog(it.appLink.appIntent, it.appLink.excludedComponents, it.appLink.uriString, it.headers) } is Command.HandleNonHttpAppLink -> { openExternalDialog(it.nonHttpAppLink.intent, it.nonHttpAppLink.fallbackUrl, false, it.headers) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index e738c6a76351..4d8db13a0c0e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -110,21 +110,21 @@ class BrowserWebViewClient( true } is SpecialUrlDetector.UrlType.AppLink -> { - Timber.i("Found app link for ${urlType.url}") + Timber.i("Found app link for ${urlType.uriString}") webViewClientListener?.let { listener -> return listener.handleAppLink(urlType, isRedirect, isForMainFrame) } false } is SpecialUrlDetector.UrlType.NonHttpAppLink -> { - Timber.i("Found non-http app link for ${urlType.url}") + Timber.i("Found non-http app link for ${urlType.uriString}") webViewClientListener?.let { listener -> return listener.handleNonHttpAppLink(urlType, isRedirect) } true } is SpecialUrlDetector.UrlType.Unknown -> { - Timber.w("Unable to process link type for ${urlType.url}") + Timber.w("Unable to process link type for ${urlType.uriString}") webView.originalUrl?.let { webView.loadUrl(it) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index a5562e3ba539..bba8c1368ac6 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -36,10 +36,10 @@ interface SpecialUrlDetector { class Telephone(val telephoneNumber: String) : UrlType() class Email(val emailAddress: String) : UrlType() class Sms(val telephoneNumber: String) : UrlType() - class AppLink(val appIntent: Intent? = null, val excludedComponents: List? = null, val url: String) : UrlType() - class NonHttpAppLink(val url: String, val intent: Intent, val fallbackUrl: String?) : UrlType() + class AppLink(val appIntent: Intent? = null, val excludedComponents: List? = null, val uriString: String) : UrlType() + class NonHttpAppLink(val uriString: String, val intent: Intent, val fallbackUrl: String?) : UrlType() class SearchQuery(val query: String) : UrlType() - class Unknown(val url: String) : UrlType() + class Unknown(val uriString: String) : UrlType() } } @@ -85,10 +85,10 @@ class SpecialUrlDetectorImpl( if (nonBrowserActivities.isNotEmpty()) { nonBrowserActivities.singleOrNull()?.let { resolveInfo -> val nonBrowserIntent = buildNonBrowserIntent(resolveInfo, uriString) - return UrlType.AppLink(appIntent = nonBrowserIntent, url = uriString) + return UrlType.AppLink(appIntent = nonBrowserIntent, uriString = uriString) } val excludedComponents = getExcludedComponents(activities) - return UrlType.AppLink(excludedComponents = excludedComponents, url = uriString) + return UrlType.AppLink(excludedComponents = excludedComponents, uriString = uriString) } } catch (e: URISyntaxException) { Timber.w(e, "Failed to parse uri $uriString") @@ -138,7 +138,7 @@ class SpecialUrlDetectorImpl( return try { val intent = Intent.parseUri(uriString, URI_NO_FLAG) val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL) - UrlType.NonHttpAppLink(url = uriString, intent = intent, fallbackUrl = fallbackUrl) + UrlType.NonHttpAppLink(uriString = uriString, intent = intent, fallbackUrl = fallbackUrl) } catch (e: URISyntaxException) { Timber.w(e, "Failed to parse uri $uriString") return UrlType.Unknown(uriString) From 2aa4749b4d4369d41d9368cb86c41e8c015f22d7 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 18:40:02 +0200 Subject: [PATCH 25/26] Fix KtLint issue --- .../main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index bba8c1368ac6..82ff833a66e5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -123,7 +123,7 @@ class SpecialUrlDetectorImpl( } private fun isBrowserInfo(resolveInfo: ResolveInfo) = - resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0 + resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0 private fun checkForIntent(scheme: String, uriString: String): UrlType { val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+") From 2b74befd51631843327a61fe945fd0808091e604 Mon Sep 17 00:00:00 2001 From: Josh Leibstein Date: Tue, 15 Jun 2021 18:48:29 +0200 Subject: [PATCH 26/26] Update function to take filter as parameter --- .../com/duckduckgo/app/browser/SpecialUrlDetector.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt index 82ff833a66e5..6a77e4f71897 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt @@ -18,6 +18,7 @@ package com.duckduckgo.app.browser import android.content.ComponentName import android.content.Intent +import android.content.IntentFilter import android.content.pm.PackageManager import android.content.pm.ResolveInfo import android.net.Uri @@ -106,7 +107,7 @@ class SpecialUrlDetectorImpl( private fun keepNonBrowserActivities(activities: List): List { return activities.filter { resolveInfo -> - resolveInfo.filter != null && !(isBrowserInfo(resolveInfo)) + resolveInfo.filter != null && !(isBrowserFilter(resolveInfo.filter)) } } @Throws(URISyntaxException::class) @@ -118,12 +119,12 @@ class SpecialUrlDetectorImpl( private fun getExcludedComponents(activities: List): List { return activities.filter { resolveInfo -> - resolveInfo.filter != null && isBrowserInfo(resolveInfo) + resolveInfo.filter != null && isBrowserFilter(resolveInfo.filter) }.map { ComponentName(it.activityInfo.packageName, it.activityInfo.name) } } - private fun isBrowserInfo(resolveInfo: ResolveInfo) = - resolveInfo.filter.countDataAuthorities() == 0 && resolveInfo.filter.countDataPaths() == 0 + private fun isBrowserFilter(filter: IntentFilter) = + filter.countDataAuthorities() == 0 && filter.countDataPaths() == 0 private fun checkForIntent(scheme: String, uriString: String): UrlType { val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+")