Merge pull request #6467 from vector-im/feature/adm/share-text-crash

Crash - Sharing text

changelog.d/6451.bugfix

@@ -0,0 +1 @@
+Fixes crash when sharing plain text, such as a url

vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt

@@ -45,7 +45,6 @@ class AttachmentsHelper(val context: Context, val callback: Callback) : Restorab
         }
 
         fun onContentAttachmentsReady(attachments: List<ContentAttachmentData>)
-        fun onAttachmentsProcessFailed()
     }
 
     // Capture path allows to handle camera image picking. It must be restored if the activity gets killed.
@@ -188,41 +187,4 @@ class AttachmentsHelper(val context: Context, val callback: Callback) : Restorab
                         .map { it.toContentAttachmentData() }
         )
     }
-
-    /**
-     * This methods aims to handle share intent.
-     *
-     * @return true if it can handle the intent data, false otherwise
-     */
-    fun handleShareIntent(context: Context, intent: Intent): Boolean {
-        val type = intent.resolveType(context) ?: return false
-        if (type.startsWith("image")) {
-            callback.onContentAttachmentsReady(
-                    MultiPicker.get(MultiPicker.IMAGE).getIncomingFiles(context, intent).map {
-                        it.toContentAttachmentData()
-                    }
-            )
-        } else if (type.startsWith("video")) {
-            callback.onContentAttachmentsReady(
-                    MultiPicker.get(MultiPicker.VIDEO).getIncomingFiles(context, intent).map {
-                        it.toContentAttachmentData()
-                    }
-            )
-        } else if (type.startsWith("audio")) {
-            callback.onContentAttachmentsReady(
-                    MultiPicker.get(MultiPicker.AUDIO).getIncomingFiles(context, intent).map {
-                        it.toContentAttachmentData()
-                    }
-            )
-        } else if (type.startsWith("application") || type.startsWith("file") || type.startsWith("text") || type.startsWith("*")) {
-            callback.onContentAttachmentsReady(
-                    MultiPicker.get(MultiPicker.FILE).getIncomingFiles(context, intent).map {
-                        it.toContentAttachmentData()
-                    }
-            )
-        } else {
-            return false
-        }
-        return true
-    }
 }

vector/src/main/java/im/vector/app/features/attachments/ShareIntentHandler.kt

@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * 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 im.vector.app.features.attachments
+
+import android.content.Context
+import android.content.Intent
+import im.vector.lib.multipicker.MultiPicker
+import org.matrix.android.sdk.api.session.content.ContentAttachmentData
+import javax.inject.Inject
+
+class ShareIntentHandler @Inject constructor() {
+
+    /**
+     * This methods aims to handle incoming share intents.
+     *
+     * @return true if it can handle the intent data, false otherwise
+     */
+    fun handleIncomingShareIntent(context: Context, intent: Intent, onFile: (List<ContentAttachmentData>) -> Unit, onPlainText: (String) -> Unit): Boolean {
+        val type = intent.resolveType(context) ?: return false
+        return when {
+            type == "text/plain" -> handlePlainText(intent, onPlainText)
+            type.startsWith("image") -> {
+                onFile(
+                        MultiPicker.get(MultiPicker.IMAGE).getIncomingFiles(context, intent).map {
+                            it.toContentAttachmentData()
+                        }
+                )
+                true
+            }
+            type.startsWith("video") -> {
+                onFile(
+                        MultiPicker.get(MultiPicker.VIDEO).getIncomingFiles(context, intent).map {
+                            it.toContentAttachmentData()
+                        }
+                )
+                true
+            }
+            type.startsWith("audio") -> {
+                onFile(
+                        MultiPicker.get(MultiPicker.AUDIO).getIncomingFiles(context, intent).map {
+                            it.toContentAttachmentData()
+                        }
+                )
+                true
+            }
+
+            type.startsWith("application") || type.startsWith("file") || type.startsWith("text") || type.startsWith("*") -> {
+                onFile(
+                        MultiPicker.get(MultiPicker.FILE).getIncomingFiles(context, intent).map {
+                            it.toContentAttachmentData()
+                        }
+                )
+                true
+            }
+            else -> false
+        }
+    }
+
+    private fun handlePlainText(intent: Intent, onPlainText: (String) -> Unit): Boolean {
+        val content = intent.getCharSequenceExtra(Intent.EXTRA_TEXT)?.toString()
+        return if (content?.isNotEmpty() == true) {
+            onPlainText(content)
+            true
+        } else {
+            false
+        }
+    }
+}

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -73,6 +73,7 @@ import im.vector.app.core.animations.play
 import im.vector.app.core.dialogs.ConfirmationDialogBuilder
 import im.vector.app.core.dialogs.GalleryOrCameraDialogHelper
 import im.vector.app.core.epoxy.LayoutManagerStateRestorer
+import im.vector.app.core.error.fatalError
 import im.vector.app.core.extensions.cleanup
 import im.vector.app.core.extensions.containsRtLOverride
 import im.vector.app.core.extensions.ensureEndsLeftToRight
@@ -130,6 +131,7 @@ import im.vector.app.features.analytics.plan.MobileScreen
 import im.vector.app.features.attachments.AttachmentTypeSelectorView
 import im.vector.app.features.attachments.AttachmentsHelper
 import im.vector.app.features.attachments.ContactAttachment
+import im.vector.app.features.attachments.ShareIntentHandler
 import im.vector.app.features.attachments.preview.AttachmentsPreviewActivity
 import im.vector.app.features.attachments.preview.AttachmentsPreviewArgs
 import im.vector.app.features.attachments.toGroupedContentAttachmentData
@@ -272,6 +274,7 @@ class TimelineFragment @Inject constructor(
         private val pillsPostProcessorFactory: PillsPostProcessor.Factory,
         private val callManager: WebRtcCallManager,
         private val audioMessagePlaybackTracker: AudioMessagePlaybackTracker,
+        private val shareIntentHandler: ShareIntentHandler,
         private val clock: Clock
 ) :
         VectorBaseFragment<FragmentTimelineBinding>(),
@@ -1616,7 +1619,9 @@ class TimelineFragment @Inject constructor(
 
     private fun sendUri(uri: Uri): Boolean {
         val shareIntent = Intent(Intent.ACTION_SEND, uri)
-        val isHandled = attachmentsHelper.handleShareIntent(requireContext(), shareIntent)
+        val isHandled = shareIntentHandler.handleIncomingShareIntent(requireContext(), shareIntent, ::onContentAttachmentsReady, onPlainText = {
+            fatalError("Should not happen as we're generating a File based share Intent", vectorPreferences.failFast())
+        })
         if (!isHandled) {
             Toast.makeText(requireContext(), R.string.error_handling_incoming_share, Toast.LENGTH_SHORT).show()
         }
@@ -2623,10 +2628,6 @@ class TimelineFragment @Inject constructor(
         }
     }
 
-    override fun onAttachmentsProcessFailed() {
-        Toast.makeText(requireContext(), R.string.error_attachment, Toast.LENGTH_SHORT).show()
-    }
-
     override fun onContactAttachmentReady(contactAttachment: ContactAttachment) {
         super.onContactAttachmentReady(contactAttachment)
         val formattedContact = contactAttachment.toHumanReadable()

vector/src/main/java/im/vector/app/features/share/IncomingShareFragment.kt

@@ -17,7 +17,6 @@
 package im.vector.app.features.share
 
 import android.app.Activity
-import android.content.ClipDescription
 import android.content.Intent
 import android.os.Bundle
 import android.view.LayoutInflater
@@ -38,10 +37,9 @@ import im.vector.app.core.extensions.registerStartForActivityResult
 import im.vector.app.core.platform.VectorBaseFragment
 import im.vector.app.databinding.FragmentIncomingShareBinding
 import im.vector.app.features.analytics.plan.ViewRoom
-import im.vector.app.features.attachments.AttachmentsHelper
+import im.vector.app.features.attachments.ShareIntentHandler
 import im.vector.app.features.attachments.preview.AttachmentsPreviewActivity
 import im.vector.app.features.attachments.preview.AttachmentsPreviewArgs
-import org.matrix.android.sdk.api.session.content.ContentAttachmentData
 import org.matrix.android.sdk.api.session.getRoomSummary
 import org.matrix.android.sdk.api.session.room.model.RoomSummary
 import javax.inject.Inject
@@ -52,13 +50,12 @@ import javax.inject.Inject
  */
 class IncomingShareFragment @Inject constructor(
         private val incomingShareController: IncomingShareController,
-        private val sessionHolder: ActiveSessionHolder
+        private val sessionHolder: ActiveSessionHolder,
+        private val shareIntentHandler: ShareIntentHandler,
 ) :
         VectorBaseFragment<FragmentIncomingShareBinding>(),
-        AttachmentsHelper.Callback,
         IncomingShareController.Callback {
 
-    private lateinit var attachmentsHelper: AttachmentsHelper
     private val viewModel: IncomingShareViewModel by fragmentViewModel()
 
     override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentIncomingShareBinding {
@@ -75,7 +72,6 @@ class IncomingShareFragment @Inject constructor(
         super.onViewCreated(view, savedInstanceState)
         setupRecyclerView()
         setupToolbar(views.incomingShareToolbar)
-        attachmentsHelper = AttachmentsHelper(requireContext(), this).register()
 
         viewModel.observeViewEvents {
             when (it) {
@@ -88,20 +84,15 @@ class IncomingShareFragment @Inject constructor(
         val intent = vectorBaseActivity.intent
         val isShareManaged = when (intent?.action) {
             Intent.ACTION_SEND -> {
-                var isShareManaged = attachmentsHelper.handleShareIntent(requireContext(), intent)
+                val isShareManaged = handleIncomingShareIntent(intent)
-                if (!isShareManaged) {
-                    isShareManaged = handleTextShare(intent)
-                }
-
                 // Direct share
                 if (intent.hasExtra(Intent.EXTRA_SHORTCUT_ID)) {
                     val roomId = intent.getStringExtra(Intent.EXTRA_SHORTCUT_ID)!!
                     sessionHolder.getSafeActiveSession()?.getRoomSummary(roomId)?.let { viewModel.handle(IncomingShareAction.ShareToRoom(it)) }
                 }
-
                 isShareManaged
             }
-            Intent.ACTION_SEND_MULTIPLE -> attachmentsHelper.handleShareIntent(requireContext(), intent)
+            Intent.ACTION_SEND_MULTIPLE -> handleIncomingShareIntent(intent)
             else -> false
         }
 
@@ -124,6 +115,19 @@ class IncomingShareFragment @Inject constructor(
         }
     }
 
+    private fun handleIncomingShareIntent(intent: Intent) = shareIntentHandler.handleIncomingShareIntent(
+            requireContext(),
+            intent,
+            onFile = {
+                val sharedData = SharedData.Attachments(it)
+                viewModel.handle(IncomingShareAction.UpdateSharedData(sharedData))
+            },
+            onPlainText = {
+                val sharedData = SharedData.Text(it)
+                viewModel.handle(IncomingShareAction.UpdateSharedData(sharedData))
+            }
+    )
+
     private fun handleMultipleRoomsShareDone(viewEvent: IncomingShareViewEvents.MultipleRoomsShareDone) {
         requireActivity().let {
             navigator.openRoom(
@@ -173,34 +177,11 @@ class IncomingShareFragment @Inject constructor(
         incomingShareController.callback = this
     }
 
-    override fun onContentAttachmentsReady(attachments: List<ContentAttachmentData>) {
-        val sharedData = SharedData.Attachments(attachments)
-        viewModel.handle(IncomingShareAction.UpdateSharedData(sharedData))
-    }
-
-    override fun onAttachmentsProcessFailed() {
-        cannotManageShare(R.string.error_handling_incoming_share)
-    }
-
     private fun cannotManageShare(@StringRes messageResId: Int) {
         Toast.makeText(requireContext(), messageResId, Toast.LENGTH_LONG).show()
         requireActivity().finish()
     }
 
-    private fun handleTextShare(intent: Intent): Boolean {
-        if (intent.type == ClipDescription.MIMETYPE_TEXT_PLAIN) {
-            val sharedText = intent.getCharSequenceExtra(Intent.EXTRA_TEXT)?.toString()
-            return if (sharedText.isNullOrEmpty()) {
-                false
-            } else {
-                val sharedData = SharedData.Text(sharedText)
-                viewModel.handle(IncomingShareAction.UpdateSharedData(sharedData))
-                true
-            }
-        }
-        return false
-    }
-
     private fun showConfirmationDialog(roomSummary: RoomSummary, sharedData: SharedData) {
         MaterialAlertDialogBuilder(requireActivity())
                 .setTitle(R.string.send_attachment)

vector/src/main/res/values/strings.xml

@@ -1814,6 +1814,8 @@
 
     <string name="error_file_too_big_simple">"The file is too large to upload."</string>
 
+    <!-- TODO Remove key -->
+    <!--suppress UnusedResources -->
     <string name="error_attachment">"An error occurred while retrieving the attachment."</string>
     <string name="attachment_type_dialog_title">"Add image from"</string>
     <string name="attachment_type_file">"File"</string>