For mozilla-mobile#26564 mozilla-mobile#27182 - Do not prematurely re…

…ad clipboard content and allow to paste non url

Fixes

Update app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt

Co-authored-by: Gabriel Luong <gabriel.luong@gmail.com>

Update app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt

Co-authored-by: Gabriel Luong <gabriel.luong@gmail.com>

fix

fix

app/src/main/java/org/mozilla/fenix/utils/ClipboardHandler.kt

@@ -38,10 +38,7 @@ class ClipboardHandler(val context: Context) {
             if (clipboard.isPrimaryClipEmpty()) {
                 return null
             }
-            if (clipboard.isPrimaryClipPlainText() ||
-                clipboard.isPrimaryClipHtmlText() ||
-                clipboard.isPrimaryClipUrlText()
-            ) {
+            if (containsText()) {
                 return firstSafePrimaryClipItemText
             }
             return null
@@ -51,7 +48,6 @@ class ClipboardHandler(val context: Context) {
         }
 
     /**
-     * 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.
@@ -68,6 +64,15 @@ class ClipboardHandler(val context: Context) {
         }
     }
 
+    /**
+     * We cannot rely on `isPrimaryClipEmpty()` since it triggers a clipboard access system notification.
+     */
+    fun containsText(): Boolean {
+        return clipboard.isPrimaryClipHtmlText() ||
+            clipboard.isPrimaryClipPlainText() ||
+            clipboard.isPrimaryClipUrlText()
+    }
+
     @Suppress("MagicNumber")
     internal fun containsURL(): Boolean {
         return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
@@ -94,6 +99,12 @@ class ClipboardHandler(val context: Context) {
     private fun ClipboardManager.isPrimaryClipUrlText() =
         primaryClipDescription?.hasMimeType(MIME_TYPE_TEXT_URL) ?: false
 
+    /**
+     * Reads the clip data, 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.
+     */
     private fun ClipboardManager.isPrimaryClipEmpty() = primaryClip?.itemCount == 0
 
     /**

app/src/main/java/org/mozilla/fenix/utils/ToolbarPopupWindow.kt

@@ -18,14 +18,17 @@ import mozilla.components.browser.state.selector.findCustomTab
 import mozilla.components.browser.state.selector.selectedTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.service.glean.private.NoExtras
-import mozilla.components.support.base.log.logger.Logger
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.databinding.BrowserToolbarPopupWindowBinding
 import org.mozilla.fenix.ext.components
 import java.lang.ref.WeakReference
 
+/**
+ * Since Android 12 reading the clipboard triggers an OS notification.
+ * As such it is important that we do not read it prematurely and only when the user trigger a paste action.
+ */
 object ToolbarPopupWindow {
     fun show(
         view: WeakReference<View>,
@@ -35,12 +38,13 @@ object ToolbarPopupWindow {
         copyVisible: Boolean = true,
     ) {
         val context = view.get()?.context ?: return
+        val isCustomTabSession = customTabId != null
         val clipboard = context.components.clipboardHandler
-        val clipboardUrl = clipboard.getUrl()
-        val clipboardText = clipboard.text
-        if (!copyVisible && clipboardUrl == null) return
 
-        val isCustomTabSession = customTabId != null
+        val containsText = clipboard.containsText()
+        val containsUrl = clipboard.containsURL()
+        val pasteDeactivated = isCustomTabSession || (!containsText && !containsUrl)
+        if (!copyVisible && pasteDeactivated) return
 
         val binding = BrowserToolbarPopupWindowBinding.inflate(LayoutInflater.from(context))
         val popupWindow = PopupWindow(
@@ -57,9 +61,8 @@ object ToolbarPopupWindow {
         popupWindow.setBackgroundDrawable(ColorDrawable(Color.TRANSPARENT))
 
         binding.copy.isVisible = copyVisible
-
-        binding.paste.isVisible = clipboardText != null && !isCustomTabSession
-        binding.pasteAndGo.isVisible = clipboardUrl != null && !isCustomTabSession
+        binding.paste.isVisible = containsText && !isCustomTabSession
+        binding.pasteAndGo.isVisible = containsUrl && !isCustomTabSession
 
         if (copyVisible) {
             binding.copy.setOnClickListener { copyView ->
@@ -82,17 +85,17 @@ object ToolbarPopupWindow {
             }
         }
 
-        clipboardText?.let { text ->
+        if (binding.paste.isVisible) {
             binding.paste.setOnClickListener {
                 popupWindow.dismiss()
-                handlePaste(text)
+                handlePaste(clipboard.text.orEmpty())
             }
         }
 
-        clipboardUrl?.let { url ->
+        if (binding.pasteAndGo.isVisible) {
             binding.pasteAndGo.setOnClickListener {
                 popupWindow.dismiss()
-                handlePasteAndGo(url)
+                handlePasteAndGo(clipboard.extractURL().orEmpty())
             }
         }
 
@@ -119,12 +122,4 @@ object ToolbarPopupWindow {
             selectedTab?.readerState?.activeUrl ?: selectedTab?.content?.url
         }
     }
-
-    private fun ClipboardHandler.getUrl(): String? {
-        if (containsURL()) {
-            text?.let { return it }
-            Logger("ToolbarPopupWindow").error("Clipboard contains URL but unable to read text")
-        }
-        return null
-    }
 }