From 8811f752e5315af2281e3f06feca39115b45d329 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 4 Jul 2022 16:15:01 +0100 Subject: [PATCH 1/4] converting if/else to when --- .../features/attachments/AttachmentsHelper.kt | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt b/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt index ffa83b608a..d17ac9d223 100644 --- a/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt +++ b/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt @@ -196,32 +196,38 @@ class AttachmentsHelper(val context: Context, val callback: Callback) : Restorab */ 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 + when { + type.startsWith("image") -> { + callback.onContentAttachmentsReady( + MultiPicker.get(MultiPicker.IMAGE).getIncomingFiles(context, intent).map { + it.toContentAttachmentData() + } + ) + } + type.startsWith("video") -> { + callback.onContentAttachmentsReady( + MultiPicker.get(MultiPicker.VIDEO).getIncomingFiles(context, intent).map { + it.toContentAttachmentData() + } + ) + } + type.startsWith("audio") -> { + callback.onContentAttachmentsReady( + MultiPicker.get(MultiPicker.AUDIO).getIncomingFiles(context, intent).map { + it.toContentAttachmentData() + } + ) + } + 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 } From dd397b9a48d8a00d042050ad7f0b59122e4a31b5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 4 Jul 2022 17:04:48 +0100 Subject: [PATCH 2/4] splitting the share intent handling from the attachments helper - decouples the attachment callback --- .../features/attachments/AttachmentsHelper.kt | 44 ---------- .../attachments/ShareIntentHandler.kt | 82 +++++++++++++++++++ .../home/room/detail/TimelineFragment.kt | 11 +-- .../features/share/IncomingShareFragment.kt | 55 ++++--------- 4 files changed, 106 insertions(+), 86 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/attachments/ShareIntentHandler.kt diff --git a/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt b/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt index d17ac9d223..d0843b3b64 100644 --- a/vector/src/main/java/im/vector/app/features/attachments/AttachmentsHelper.kt +++ b/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) - fun onAttachmentsProcessFailed() } // Capture path allows to handle camera image picking. It must be restored if the activity gets killed. @@ -188,47 +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 - when { - type.startsWith("image") -> { - callback.onContentAttachmentsReady( - MultiPicker.get(MultiPicker.IMAGE).getIncomingFiles(context, intent).map { - it.toContentAttachmentData() - } - ) - } - type.startsWith("video") -> { - callback.onContentAttachmentsReady( - MultiPicker.get(MultiPicker.VIDEO).getIncomingFiles(context, intent).map { - it.toContentAttachmentData() - } - ) - } - type.startsWith("audio") -> { - callback.onContentAttachmentsReady( - MultiPicker.get(MultiPicker.AUDIO).getIncomingFiles(context, intent).map { - it.toContentAttachmentData() - } - ) - } - 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 - } } diff --git a/vector/src/main/java/im/vector/app/features/attachments/ShareIntentHandler.kt b/vector/src/main/java/im/vector/app/features/attachments/ShareIntentHandler.kt new file mode 100644 index 0000000000..06ca949025 --- /dev/null +++ b/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) -> 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 + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index 855df14e60..fd1324d653 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt @@ -74,6 +74,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(), @@ -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() diff --git a/vector/src/main/java/im/vector/app/features/share/IncomingShareFragment.kt b/vector/src/main/java/im/vector/app/features/share/IncomingShareFragment.kt index 6baedcd125..a113c8105d 100644 --- a/vector/src/main/java/im/vector/app/features/share/IncomingShareFragment.kt +++ b/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(), - 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) - if (!isShareManaged) { - isShareManaged = handleTextShare(intent) - } - + val isShareManaged = handleIncomingShareIntent(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) { - 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) From 3c092c4e2a1818d4863ba48897c2873677d51b8b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Jul 2022 10:37:41 +0100 Subject: [PATCH 3/4] adding changelog entry --- changelog.d/6451.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6451.bugfix diff --git a/changelog.d/6451.bugfix b/changelog.d/6451.bugfix new file mode 100644 index 0000000000..ed4a2df7e8 --- /dev/null +++ b/changelog.d/6451.bugfix @@ -0,0 +1 @@ +Fixes crash when sharing plain text, such as a url From 03202080b354289b6ef33d8648fc9ceab74f74c3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Jul 2022 11:17:10 +0100 Subject: [PATCH 4/4] suppressing unused string resource --- vector/src/main/res/values/strings.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vector/src/main/res/values/strings.xml b/vector/src/main/res/values/strings.xml index 2a9cdb832d..5c7371ea2f 100644 --- a/vector/src/main/res/values/strings.xml +++ b/vector/src/main/res/values/strings.xml @@ -1814,6 +1814,8 @@ "The file is too large to upload." + + "An error occurred while retrieving the attachment." "Add image from" "File"