Merge pull request #2373 from vector-im/feature/bma/fix_crash

Feature/bma/fix crash

CHANGES.md

@@ -13,6 +13,7 @@ Improvements 🙌:
  - Move "Enable Encryption" from room setting screen to room profile screen (#2394)
 
 Bugfix 🐛:
+ - Fix crash on AttachmentViewer (#2365)
  - Exclude yourself when decorating rooms which are direct or don't have more than 2 users (#2370)
  - F-Droid version: ensure timeout of sync request can be more than 60 seconds (#2169)
  - Fix issue when restoring draft after sharing (#2287)

vector/src/main/java/im/vector/app/features/media/AttachmentProviderFactory.kt

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2020 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.media
+
+import im.vector.app.core.date.VectorDateFormatter
+import im.vector.app.core.resources.StringProvider
+import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.session.room.Room
+import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
+import javax.inject.Inject
+
+class AttachmentProviderFactory @Inject constructor(
+        private val imageContentRenderer: ImageContentRenderer,
+        private val vectorDateFormatter: VectorDateFormatter,
+        private val stringProvider: StringProvider,
+        private val session: Session
+) {
+
+    fun createProvider(attachments: List<TimelineEvent>): RoomEventsAttachmentProvider {
+        return RoomEventsAttachmentProvider(
+                attachments,
+                imageContentRenderer,
+                vectorDateFormatter,
+                session.fileService(),
+                stringProvider
+        )
+    }
+
+    fun createProvider(attachments: List<AttachmentData>, room: Room?): DataAttachmentRoomProvider {
+        return DataAttachmentRoomProvider(
+                attachments,
+                room,
+                imageContentRenderer,
+                vectorDateFormatter,
+                session.fileService(),
+                stringProvider
+        )
+    }
+}

vector/src/main/java/im/vector/app/features/media/BaseAttachmentProvider.kt

@@ -20,17 +20,30 @@ import android.content.Context
 import android.graphics.drawable.Drawable
 import android.view.View
 import android.widget.ImageView
+import androidx.core.view.isVisible
 import com.bumptech.glide.request.target.CustomViewTarget
 import com.bumptech.glide.request.transition.Transition
+import im.vector.app.R
+import im.vector.app.core.date.DateFormatKind
+import im.vector.app.core.date.VectorDateFormatter
+import im.vector.app.core.resources.StringProvider
 import im.vector.lib.attachmentviewer.AttachmentInfo
 import im.vector.lib.attachmentviewer.AttachmentSourceProvider
 import im.vector.lib.attachmentviewer.ImageLoaderTarget
 import im.vector.lib.attachmentviewer.VideoLoaderTarget
 import org.matrix.android.sdk.api.MatrixCallback
+import org.matrix.android.sdk.api.session.events.model.isVideoMessage
 import org.matrix.android.sdk.api.session.file.FileService
+import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import java.io.File
 
-abstract class BaseAttachmentProvider(val imageContentRenderer: ImageContentRenderer, val fileService: FileService) : AttachmentSourceProvider {
+abstract class BaseAttachmentProvider<Type>(
+        private val attachments: List<Type>,
+        private val imageContentRenderer: ImageContentRenderer,
+        protected val fileService: FileService,
+        private val dateFormatter: VectorDateFormatter,
+        private val stringProvider: StringProvider
+) : AttachmentSourceProvider {
 
     interface InteractionListener {
         fun onDismissTapped()
@@ -41,9 +54,13 @@ abstract class BaseAttachmentProvider(val imageContentRenderer: ImageContentRend
 
     var interactionListener: InteractionListener? = null
 
-    protected var overlayView: AttachmentOverlayView? = null
+    private var overlayView: AttachmentOverlayView? = null
 
-    override fun overlayViewAtPosition(context: Context, position: Int): View? {
+    final override fun getItemCount() = attachments.size
+
+    protected fun getItem(position: Int) = attachments[position]
+
+    final override fun overlayViewAtPosition(context: Context, position: Int): View? {
         if (position == -1) return null
         if (overlayView == null) {
             overlayView = AttachmentOverlayView(context)
@@ -60,9 +77,24 @@ abstract class BaseAttachmentProvider(val imageContentRenderer: ImageContentRend
                 interactionListener?.videoSeekTo(percent)
             }
         }
+
+        val timelineEvent = getTimelineEventAtPosition(position)
+        if (timelineEvent != null) {
+            val dateString = dateFormatter.format(timelineEvent.root.originServerTs, DateFormatKind.DEFAULT_DATE_AND_TIME)
+            overlayView?.updateWith(
+                    counter = stringProvider.getString(R.string.attachment_viewer_item_x_of_y, position + 1, getItemCount()),
+                    senderInfo = "${timelineEvent.senderInfo.displayName} $dateString"
+            )
+            overlayView?.videoControlsGroup?.isVisible = timelineEvent.root.isVideoMessage()
+        } else {
+            overlayView?.updateWith("", "")
+        }
+
         return overlayView
     }
 
+    abstract fun getTimelineEventAtPosition(position: Int): TimelineEvent?
+
     override fun loadImage(target: ImageLoaderTarget, info: AttachmentInfo.Image) {
         (info.data as? ImageContentRenderer.Data)?.let {
             imageContentRenderer.render(it, target.contextView(), object : CustomViewTarget<ImageView, Drawable>(target.contextView()) {

vector/src/main/java/im/vector/app/features/media/DataAttachmentRoomProvider.kt

@@ -16,30 +16,26 @@
 
 package im.vector.app.features.media
 
-import android.content.Context
-import android.view.View
-import androidx.core.view.isVisible
-import im.vector.app.core.date.DateFormatKind
 import im.vector.app.core.date.VectorDateFormatter
+import im.vector.app.core.resources.StringProvider
 import im.vector.lib.attachmentviewer.AttachmentInfo
 import org.matrix.android.sdk.api.MatrixCallback
-import org.matrix.android.sdk.api.session.events.model.isVideoMessage
 import org.matrix.android.sdk.api.session.file.FileService
 import org.matrix.android.sdk.api.session.room.Room
+import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import java.io.File
 
 class DataAttachmentRoomProvider(
-        private val attachments: List<AttachmentData>,
+        attachments: List<AttachmentData>,
         private val room: Room?,
-        private val initialIndex: Int,
         imageContentRenderer: ImageContentRenderer,
-        private val dateFormatter: VectorDateFormatter,
+        dateFormatter: VectorDateFormatter,
-        fileService: FileService) : BaseAttachmentProvider(imageContentRenderer, fileService) {
+        fileService: FileService,
-
+        stringProvider: StringProvider
-    override fun getItemCount(): Int = attachments.size
+) : BaseAttachmentProvider<AttachmentData>(attachments, imageContentRenderer, fileService, dateFormatter, stringProvider) {
 
     override fun getAttachmentInfoAt(position: Int): AttachmentInfo {
-        return attachments[position].let {
+        return getItem(position).let {
             when (it) {
                 is ImageContentRenderer.Data -> {
                     if (it.mimeType == "image/gif") {
@@ -73,22 +69,13 @@ class DataAttachmentRoomProvider(
         }
     }
 
-    override fun overlayViewAtPosition(context: Context, position: Int): View? {
+    override fun getTimelineEventAtPosition(position: Int): TimelineEvent? {
-        super.overlayViewAtPosition(context, position)
+        val item = getItem(position)
-        val item = attachments[position]
+        return room?.getTimeLineEvent(item.eventId)
-        val timeLineEvent = room?.getTimeLineEvent(item.eventId)
-        if (timeLineEvent != null) {
-            val dateString = dateFormatter.format(timeLineEvent.root.originServerTs, DateFormatKind.DEFAULT_DATE_AND_TIME)
-            overlayView?.updateWith("${position + 1} of ${attachments.size}", "${timeLineEvent.senderInfo.displayName} $dateString")
-            overlayView?.videoControlsGroup?.isVisible = timeLineEvent.root.isVideoMessage()
-        } else {
-            overlayView?.updateWith("", "")
-        }
-        return overlayView
     }
 
     override fun getFileForSharing(position: Int, callback: (File?) -> Unit) {
-        val item = attachments[position]
+        val item = getItem(position)
         fileService.downloadFile(
                 downloadMode = FileService.DownloadMode.FOR_EXTERNAL_SHARE,
                 id = item.eventId,

vector/src/main/java/im/vector/app/features/media/RoomEventsAttachmentProvider.kt

@@ -16,18 +16,12 @@
 
 package im.vector.app.features.media
 
-import android.content.Context
-import android.view.View
-import androidx.core.view.isVisible
-import im.vector.app.core.date.DateFormatKind
 import im.vector.app.core.date.VectorDateFormatter
+import im.vector.app.core.resources.StringProvider
 import im.vector.lib.attachmentviewer.AttachmentInfo
 import org.matrix.android.sdk.api.MatrixCallback
-import org.matrix.android.sdk.api.session.Session
-import org.matrix.android.sdk.api.session.events.model.isVideoMessage
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.file.FileService
-import org.matrix.android.sdk.api.session.room.Room
 import org.matrix.android.sdk.api.session.room.model.message.MessageContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageImageContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageVideoContent
@@ -36,22 +30,17 @@ import org.matrix.android.sdk.api.session.room.model.message.getFileUrl
 import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import org.matrix.android.sdk.internal.crypto.attachments.toElementToDecrypt
 import java.io.File
-import javax.inject.Inject
 
 class RoomEventsAttachmentProvider(
-        private val attachments: List<TimelineEvent>,
+        attachments: List<TimelineEvent>,
-        private val initialIndex: Int,
         imageContentRenderer: ImageContentRenderer,
-        private val dateFormatter: VectorDateFormatter,
+        dateFormatter: VectorDateFormatter,
-        fileService: FileService
+        fileService: FileService,
-) : BaseAttachmentProvider(imageContentRenderer, fileService) {
+        stringProvider: StringProvider
-
+) : BaseAttachmentProvider<TimelineEvent>(attachments, imageContentRenderer, fileService, dateFormatter, stringProvider) {
-    override fun getItemCount(): Int {
-        return attachments.size
-    }
 
     override fun getAttachmentInfoAt(position: Int): AttachmentInfo {
-        return attachments[position].let {
+        return getItem(position).let {
             val content = it.root.getClearContent().toModel<MessageContent>() as? MessageWithAttachmentContent
             if (content is MessageImageContent) {
                 val data = ImageContentRenderer.Data(
@@ -125,17 +114,12 @@ class RoomEventsAttachmentProvider(
         }
     }
 
-    override fun overlayViewAtPosition(context: Context, position: Int): View? {
+    override fun getTimelineEventAtPosition(position: Int): TimelineEvent? {
-        super.overlayViewAtPosition(context, position)
+        return getItem(position)
-        val item = attachments[position]
-        val dateString = dateFormatter.format(item.root.originServerTs, DateFormatKind.DEFAULT_DATE_AND_TIME)
-        overlayView?.updateWith("${position + 1} of ${attachments.size}", "${item.senderInfo.displayName} $dateString")
-        overlayView?.videoControlsGroup?.isVisible = item.root.isVideoMessage()
-        return overlayView
     }
 
     override fun getFileForSharing(position: Int, callback: (File?) -> Unit) {
-        attachments[position].let { timelineEvent ->
+        getItem(position).let { timelineEvent ->
 
             val messageContent = timelineEvent.root.getClearContent().toModel<MessageContent>()
                     as? MessageWithAttachmentContent
@@ -160,18 +144,3 @@ class RoomEventsAttachmentProvider(
         }
     }
 }
-
-class AttachmentProviderFactory @Inject constructor(
-        private val imageContentRenderer: ImageContentRenderer,
-        private val vectorDateFormatter: VectorDateFormatter,
-        private val session: Session
-) {
-
-    fun createProvider(attachments: List<TimelineEvent>, initialIndex: Int): RoomEventsAttachmentProvider {
-        return RoomEventsAttachmentProvider(attachments, initialIndex, imageContentRenderer, vectorDateFormatter, session.fileService())
-    }
-
-    fun createProvider(attachments: List<AttachmentData>, room: Room?, initialIndex: Int): DataAttachmentRoomProvider {
-        return DataAttachmentRoomProvider(attachments, room, initialIndex, imageContentRenderer, vectorDateFormatter, session.fileService())
-    }
-}

vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt

@@ -70,7 +70,7 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen
     private var initialIndex = 0
     private var isAnimatingOut = false
 
-    var currentSourceProvider: BaseAttachmentProvider? = null
+    private var currentSourceProvider: BaseAttachmentProvider<*>? = null
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -117,36 +117,22 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen
         val room = args.roomId?.let { session.getRoom(it) }
 
         val inMemoryData = intent.getParcelableArrayListExtra<AttachmentData>(EXTRA_IN_MEMORY_DATA)
-        if (inMemoryData != null) {
+        val sourceProvider = if (inMemoryData != null) {
-            val sourceProvider = dataSourceFactory.createProvider(inMemoryData, room, initialIndex)
+            initialIndex = inMemoryData.indexOfFirst { it.eventId == args.eventId }.coerceAtLeast(0)
-            val index = inMemoryData.indexOfFirst { it.eventId == args.eventId }
+            dataSourceFactory.createProvider(inMemoryData, room)
-            initialIndex = index
-            sourceProvider.interactionListener = this
-            setSourceProvider(sourceProvider)
-            this.currentSourceProvider = sourceProvider
-            if (savedInstanceState == null) {
-                pager2.setCurrentItem(index, false)
-                // The page change listener is not notified of the change...
-                pager2.post {
-                    onSelectedPositionChanged(index)
-                }
-            }
         } else {
-            val events = room?.getAttachmentMessages()
+            val events = room?.getAttachmentMessages().orEmpty()
-                    ?: emptyList()
+            initialIndex = events.indexOfFirst { it.eventId == args.eventId }.coerceAtLeast(0)
-            val index = events.indexOfFirst { it.eventId == args.eventId }
+            dataSourceFactory.createProvider(events)
-            initialIndex = index
+        }
-
+        sourceProvider.interactionListener = this
-            val sourceProvider = dataSourceFactory.createProvider(events, index)
+        setSourceProvider(sourceProvider)
-            sourceProvider.interactionListener = this
+        currentSourceProvider = sourceProvider
-            setSourceProvider(sourceProvider)
+        if (savedInstanceState == null) {
-            this.currentSourceProvider = sourceProvider
+            pager2.setCurrentItem(initialIndex, false)
-            if (savedInstanceState == null) {
+            // The page change listener is not notified of the change...
-                pager2.setCurrentItem(index, false)
+            pager2.post {
-                // The page change listener is not notified of the change...
+                onSelectedPositionChanged(initialIndex)
-                pager2.post {
-                    onSelectedPositionChanged(index)
-                }
             }
         }
 
@@ -278,7 +264,7 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen
     }
 
     override fun onShareTapped() {
-        this.currentSourceProvider?.getFileForSharing(currentPosition) { data ->
+        currentSourceProvider?.getFileForSharing(currentPosition) { data ->
             if (data != null && lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) {
                 shareMedia(this@VectorAttachmentViewerActivity, data, getMimeTypeFromUri(this@VectorAttachmentViewerActivity, data.toUri()))
             }

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

@@ -1860,6 +1860,8 @@
     <string name="rotate_and_crop_screen_title">Rotate and crop</string>
     <string name="error_handling_incoming_share">Couldn\'t handle share data</string>
 
+    <string name="attachment_viewer_item_x_of_y">%1$d of %2$d</string>
+
     <string name="uploads_media_title">MEDIA</string>
     <string name="uploads_media_no_result">There are no media in this room</string>
     <string name="uploads_files_title">FILES</string>