Merge pull request #471 from RyanRory/conversation-scroll-issues

Fix Conversation Screen Scrolling & Marking Messages as Read

Session/Conversations/ConversationVC+Interaction.swift

@@ -18,7 +18,10 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc
     }
 
     func handleScrollToBottomButtonTapped() {
-        scrollToBottom(isAnimated: true)
+        // The tableview's content size will be calculated by the estimated height of cells,
+        // so the result may be inaccurate before all the cells are loaded.
+        // Use this scroll to the last row instead.
+        messagesTableView.scrollToRow(at: IndexPath(row: viewItems.count-1, section: 0), at: .top, animated: true)
     }
 
     // MARK: Blocking

Session/Conversations/ConversationVC.swift

@@ -222,8 +222,13 @@ final class ConversationVC : BaseVC, ConversationViewModelDelegate, OWSConversat
             Storage.read { transaction in
                 unreadCount = self.thread.unreadMessageCount(transaction: transaction)
             }
+            // When the unread message count is more than the number of a page of viewItems,
+            // the screen will scroll to bottom instead of the correct first unread message.
+            // The unreadIndicatorIndex is calculated during loading the viewItems, it is
+            // supposed to be accurate.
             DispatchQueue.main.async {
-                if unreadCount > 0, let viewItem = self.viewItems[ifValid: self.viewItems.count - Int(unreadCount)], let interactionID = viewItem.interaction.uniqueId {
+                let unreadIndicatorIndex = self.viewModel.viewState.unreadIndicatorIndex?.intValue ?? (self.viewItems.count - self.unreadViewItems.count)
+                if unreadCount > 0, let viewItem = self.viewItems[ifValid: unreadIndicatorIndex], let interactionID = viewItem.interaction.uniqueId {
                     self.scrollToInteraction(with: interactionID, position: .top, isAnimated: false)
                     self.unreadCountView.alpha = self.scrollButton.alpha
                 } else {

SessionMessagingKit/Jobs/AttachmentDownloadJob.swift

@@ -86,6 +86,11 @@ public final class AttachmentDownloadJob : NSObject, Job, NSCoding { // NSObject
                 self.handlePermanentFailure(error: error)
             } else if let error = error as? OnionRequestAPI.Error, case .httpRequestFailedAtDestination(let statusCode, _, _) = error,
                 statusCode == 400 {
+                // Otherwise, the attachment will show a state of downloading forever,
+                // and the message won't be able to be marked as read.
+                storage.write(with: { transaction in
+                    storage.setAttachmentState(to: .failed, for: pointer, associatedWith: self.tsMessageID, using: transaction)
+                }, completion: { })
                 // This usually indicates a file that has expired on the server, so there's no need to retry.
                 self.handlePermanentFailure(error: error)
             } else {

SessionMessagingKit/Messages/Signal/TSIncomingMessage.m

@@ -147,6 +147,11 @@ NS_ASSUME_NONNULL_BEGIN
     BOOL areAllAttachmentsDownloaded = YES;
     for (NSString *attachmentId in self.attachmentIds) {
         TSAttachment *attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId transaction:transaction];
+        // If the attachment download is failed, we can mark this message as read.
+        // Otherwise, this message will never be marked as read.
+        if ([attachment isKindOfClass:[TSAttachmentPointer class]] && ((TSAttachmentPointer *)attachment).state == TSAttachmentPointerStateFailed) {
+            continue;
+        }
         areAllAttachmentsDownloaded = areAllAttachmentsDownloaded && attachment.isDownloaded;
         if (!areAllAttachmentsDownloaded) break;
     }