From 94a543a4032feac18a90e3d1449587631a9595fe Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Tue, 9 Nov 2021 18:46:30 -0500 Subject: [PATCH] For #22271 Improve URL accessing from the clipboard for Android 12 and above. --- .../fenix/ui/robots/NavigationToolbarRobot.kt | 14 ++-- .../fenix/search/SearchDialogFragment.kt | 66 ++++++++++++------- .../fenix/search/SearchFragmentStore.kt | 10 +-- .../mozilla/fenix/utils/ClipboardHandler.kt | 44 +++++++++++-- .../mozilla/fenix/utils/ToolbarPopupWindow.kt | 6 +- app/src/main/res/values-v31/dimens.xml | 7 ++ .../fenix/search/SearchFragmentStoreTest.kt | 9 +-- .../fenix/utils/ClipboardHandlerTest.kt | 8 +-- 8 files changed, 113 insertions(+), 51 deletions(-) create mode 100644 app/src/main/res/values-v31/dimens.xml diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/NavigationToolbarRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/NavigationToolbarRobot.kt index 4cdfbd0ffc..cad4b23a46 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/NavigationToolbarRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/NavigationToolbarRobot.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.ui.robots import android.net.Uri +import android.os.Build import androidx.recyclerview.widget.RecyclerView import androidx.test.espresso.Espresso.onView import androidx.test.espresso.IdlingRegistry @@ -162,10 +163,15 @@ class NavigationToolbarRobot { waitingTime ) - mDevice.waitNotNull( - Until.findObject(By.res("org.mozilla.fenix.debug:id/clipboard_url")), - waitingTime - ) + // On Android 12 or above we don't SHOW the URL unless the user requests to do so. + // See for mor information https://github.com/mozilla-mobile/fenix/issues/22271 + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S) { + mDevice.waitNotNull( + Until.findObject(By.res("org.mozilla.fenix.debug:id/clipboard_url")), + waitingTime + ) + } + fillLinkButton().click() BrowserRobot().interact() diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index 8c3ef5eb54..7ad6b9eb56 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -288,14 +288,22 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { binding.fillLinkFromClipboard.setOnClickListener { requireComponents.analytics.metrics.track(Event.ClipboardSuggestionClicked) - view.hideKeyboard() - toolbarView.view.clearFocus() - (activity as HomeActivity) - .openToBrowserAndLoad( - searchTermOrURL = requireContext().components.clipboardHandler.url ?: "", - newTab = store.state.tabId == null, - from = BrowserDirection.FromSearchDialog - ) + val clipboardUrl = requireContext().components.clipboardHandler.extractURL() ?: "" + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + toolbarView.view.edit.updateUrl(clipboardUrl) + hideClipboardSection() + inlineAutocompleteEditText.setSelection(clipboardUrl.length) + } else { + view.hideKeyboard() + toolbarView.view.clearFocus() + (activity as HomeActivity) + .openToBrowserAndLoad( + searchTermOrURL = clipboardUrl, + newTab = store.state.tabId == null, + from = BrowserDirection.FromSearchDialog + ) + } } val stubListener = ViewStub.OnInflateListener { _, inflated -> @@ -356,6 +364,15 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { } } + private fun hideClipboardSection() { + binding.fillLinkFromClipboard.isVisible = false + binding.fillLinkDivider.isVisible = false + binding.pillWrapperDivider.isVisible = false + binding.clipboardUrl.isVisible = false + binding.clipboardTitle.isVisible = false + binding.linkIcon.isVisible = false + } + private fun observeSuggestionProvidersState() = consumeFlow(store) { flow -> flow.map { state -> state.toSearchProviderState() } .ifChanged() @@ -389,12 +406,12 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { flow.map { state -> val shouldShowView = state.showClipboardSuggestions && state.query.isEmpty() && - !state.clipboardUrl.isNullOrEmpty() && !state.showSearchShortcuts - Pair(shouldShowView, state.clipboardUrl) + state.clipboardHasUrl && !state.showSearchShortcuts + Pair(shouldShowView, state.clipboardHasUrl) } .ifChanged() - .collect { (shouldShowView, clipboardUrl) -> - updateClipboardSuggestion(shouldShowView, clipboardUrl) + .collect { (shouldShowView) -> + updateClipboardSuggestion(shouldShowView) } } @@ -418,9 +435,8 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { // We delay querying the clipboard by posting this code to the main thread message queue, // because ClipboardManager will return null if the does app not have input focus yet. lifecycleScope.launch(Dispatchers.Cached) { - context?.components?.clipboardHandler?.url?.let { clipboardUrl -> - store.dispatch(SearchFragmentAction.UpdateClipboardUrl(clipboardUrl)) - } + val hasUrl = context?.components?.clipboardHandler?.containsURL() ?: false + store.dispatch(SearchFragmentAction.UpdateClipboardHasUrl(hasUrl)) } } } @@ -655,25 +671,29 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { private fun isSpeechAvailable(): Boolean = speechIntent.resolveActivity(requireContext().packageManager) != null private fun updateClipboardSuggestion( - shouldShowView: Boolean, - clipboardUrl: String? + shouldShowView: Boolean ) { binding.fillLinkFromClipboard.isVisible = shouldShowView binding.fillLinkDivider.isVisible = shouldShowView binding.pillWrapperDivider.isVisible = !(shouldShowView && requireComponents.settings.shouldUseBottomToolbar) - binding.clipboardUrl.isVisible = shouldShowView binding.clipboardTitle.isVisible = shouldShowView binding.linkIcon.isVisible = shouldShowView - binding.clipboardUrl.text = clipboardUrl + val contentDescription = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + "${binding.clipboardTitle.text}." + } else { + val clipboardUrl = context?.components?.clipboardHandler?.extractURL() - binding.fillLinkFromClipboard.contentDescription = + if (clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) { + requireComponents.core.engine.speculativeConnect(clipboardUrl) + } + binding.clipboardUrl.text = clipboardUrl + binding.clipboardUrl.isVisible = shouldShowView "${binding.clipboardTitle.text}, ${binding.clipboardUrl.text}." - - if (clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) { - requireComponents.core.engine.speculativeConnect(clipboardUrl) } + + binding.fillLinkFromClipboard.contentDescription = contentDescription } private fun updateToolbarContentDescription(source: SearchEngineSource) { diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt index 0db9b845fa..4649994482 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt @@ -61,7 +61,7 @@ sealed class SearchEngineSource { * @property showHistorySuggestions Whether or not to show history suggestions in the AwesomeBar * @property showBookmarkSuggestions Whether or not to show the bookmark suggestion in the AwesomeBar * @property pastedText The text pasted from the long press toolbar menu - * @property clipboardUrl The URL in the clipboard of the user - if there's any; otherwise `null`. + * @property clipboardHasUrl Indicates if the clipboard contains an URL. */ data class SearchFragmentState( val query: String, @@ -81,7 +81,7 @@ data class SearchFragmentState( val tabId: String?, val pastedText: String? = null, val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint?, - val clipboardUrl: String? = null + val clipboardHasUrl: Boolean = false ) : State fun createInitialSearchFragmentState( @@ -131,7 +131,7 @@ sealed class SearchFragmentAction : Action { data class ShowSearchShortcutEnginePicker(val show: Boolean) : SearchFragmentAction() data class AllowSearchSuggestionsInPrivateModePrompt(val show: Boolean) : SearchFragmentAction() data class UpdateQuery(val query: String) : SearchFragmentAction() - data class UpdateClipboardUrl(val url: String?) : SearchFragmentAction() + data class UpdateClipboardHasUrl(val hasUrl: Boolean) : SearchFragmentAction() /** * Updates the local `SearchFragmentState` from the global `SearchState` in `BrowserStore`. @@ -172,9 +172,9 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen } ) } - is SearchFragmentAction.UpdateClipboardUrl -> { + is SearchFragmentAction.UpdateClipboardHasUrl -> { state.copy( - clipboardUrl = action.url + clipboardHasUrl = action.hasUrl ) } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt b/app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt index e479c15c11..eac1660d60 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt @@ -7,6 +7,8 @@ package org.mozilla.fenix.utils import android.content.ClipData import android.content.ClipboardManager import android.content.Context +import android.os.Build +import android.view.textclassifier.TextClassifier import androidx.annotation.VisibleForTesting import androidx.core.content.getSystemService import mozilla.components.support.utils.SafeUrl @@ -21,6 +23,13 @@ private const val MIME_TYPE_TEXT_HTML = "text/html" class ClipboardHandler(val context: Context) { private val clipboard = context.getSystemService()!! + /** + * Provides access to the current content of the clipboard, be aware this is a sensitive + * API as from Android 12 and above, accessing it will trigger a notification letting the user + * know the app has accessed the clipboard, make sure when you call this API that users are + * completely aware that we are accessing the clipboard. + * See for more details https://github.com/mozilla-mobile/fenix/issues/22271. + */ var text: String? get() { if (!clipboard.isPrimaryClipEmpty() && @@ -37,13 +46,36 @@ class ClipboardHandler(val context: Context) { clipboard.setPrimaryClip(ClipData.newPlainText("Text", value)) } - val url: String? - get() { - return text?.let { - val finder = WebURLFinder(it) - finder.bestWebURL() - } + /** + * Returns a possible URL from the actual content of the clipboard, be aware this is a sensitive + * API as from Android 12 and above, accessing it will trigger a notification letting the user + * know the app has accessed the clipboard, make sure when you call this API that users are + * completely aware that we are accessing the clipboard. + * See for more details https://github.com/mozilla-mobile/fenix/issues/22271. + */ + fun extractURL(): String? { + return text?.let { + val finder = WebURLFinder(it) + finder.bestWebURL() } + } + + @Suppress("MagicNumber") + internal fun containsURL(): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + val description = clipboard.primaryClipDescription + // An IllegalStateException is thrown if the url is too long. + val score = + try { + description?.getConfidenceScore(TextClassifier.TYPE_URL) ?: 0F + } catch (e: IllegalStateException) { + 0F + } + score >= 0.7F + } else { + !extractURL().isNullOrEmpty() + } + } private fun ClipboardManager.isPrimaryClipPlainText() = primaryClipDescription?.hasMimeType(MIME_TYPE_TEXT_PLAIN) ?: false diff --git a/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt b/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt index da0cadc58d..8c91eeb0bb 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt @@ -34,7 +34,7 @@ object ToolbarPopupWindow { ) { val context = view.get()?.context ?: return val clipboard = context.components.clipboardHandler - if (!copyVisible && clipboard.text.isNullOrEmpty()) return + if (!copyVisible && !clipboard.containsURL()) return val isCustomTabSession = customTabId != null @@ -54,9 +54,9 @@ object ToolbarPopupWindow { binding.copy.isVisible = copyVisible - binding.paste.isVisible = !clipboard.text.isNullOrEmpty() && !isCustomTabSession + binding.paste.isVisible = clipboard.containsURL() && !isCustomTabSession binding.pasteAndGo.isVisible = - !clipboard.text.isNullOrEmpty() && !isCustomTabSession + clipboard.containsURL() && !isCustomTabSession binding.copy.setOnClickListener { popupWindow.dismiss() diff --git a/app/src/main/res/values-v31/dimens.xml b/app/src/main/res/values-v31/dimens.xml new file mode 100644 index 0000000000..fa80d2708c --- /dev/null +++ b/app/src/main/res/values-v31/dimens.xml @@ -0,0 +1,7 @@ + + + + 49dp + diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt index 2e835b91d9..65e056775a 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt @@ -203,16 +203,13 @@ class SearchFragmentStoreTest { val initialState = emptyDefaultState() val store = SearchFragmentStore(initialState) - assertNull(store.state.clipboardUrl) + assertFalse(store.state.clipboardHasUrl) store.dispatch( - SearchFragmentAction.UpdateClipboardUrl("https://www.mozilla.org") + SearchFragmentAction.UpdateClipboardHasUrl(true) ).joinBlocking() - assertEquals( - "https://www.mozilla.org", - store.state.clipboardUrl - ) + assertTrue(store.state.clipboardHasUrl) } @Test diff --git a/app/src/test/java/org/mozilla/fenix/utils/ClipboardHandlerTest.kt b/app/src/test/java/org/mozilla/fenix/utils/ClipboardHandlerTest.kt index b70c8e6246..5161fc1616 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/ClipboardHandlerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/ClipboardHandlerTest.kt @@ -52,18 +52,18 @@ class ClipboardHandlerTest { @Test fun getUrl() { - assertEquals(null, clipboardHandler.url) + assertEquals(null, clipboardHandler.extractURL()) clipboard.setPrimaryClip(ClipData.newPlainText("Text", clipboardUrl)) - assertEquals(clipboardUrl, clipboardHandler.url) + assertEquals(clipboardUrl, clipboardHandler.extractURL()) } @Test fun getUrlfromTextUrlMIME() { - assertEquals(null, clipboardHandler.url) + assertEquals(null, clipboardHandler.extractURL()) clipboard.setPrimaryClip(ClipData.newHtmlText("Html", clipboardUrl, clipboardUrl)) - assertEquals(clipboardUrl, clipboardHandler.url) + assertEquals(clipboardUrl, clipboardHandler.extractURL()) } @Test