BugFix - Chat screen: wrong attachment is opened.

element-hq/element-ios#237
matrix-org · Apr 22, 2016 · d340ef0 · d340ef0
1 parent 32d5b94
commit d340ef0
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 15 deletions.
diff --git a/MatrixKit/Controllers/MXKAttachmentsViewController.m b/MatrixKit/Controllers/MXKAttachmentsViewController.m
@@ -28,6 +28,8 @@
 
 #import "NSBundle+MatrixKit.h"
 
+#import "MXKEventFormatter.h"
+
 @interface MXKAttachmentsViewController ()
 {
     /**
@@ -150,6 +152,9 @@ - (void)viewWillAppear:(BOOL)animated
     }
 
     [_attachmentsCollection reloadData];
+
+    // Adjust content offset
+    [self refreshAttachmentCollectionContentOffset];
 }
 
 - (void)viewDidAppear:(BOOL)animated
@@ -282,6 +287,7 @@ - (void)destroy
 - (void)displayAttachments:(NSArray*)attachmentArray focusOn:(NSString*)eventId
 {
     NSString *currentAttachmentEventId = eventId;
+    NSString *currentAttachmentOriginalFileName = nil;
 
     if (currentAttachmentEventId.length == 0 && attachments)
     {
@@ -296,11 +302,23 @@ - (void)displayAttachments:(NSArray*)attachmentArray focusOn:(NSString*)eventId
                 // Retrieve the event id of the first item in the current attachments array
                 MXKAttachment *attachment = attachments[0];
                 NSString *firstAttachmentEventId = attachment.event.eventId;
+                NSString *firstAttachmentOriginalFileName = nil;
+
+                // The original file name is used when the attachment is a local echo.
+                // Indeed its event id may be replaced by the actual one in the new attachments array.
+                if ([firstAttachmentEventId hasPrefix:kMXKEventFormatterLocalEventIdPrefix])
+                {
+                    firstAttachmentOriginalFileName = attachment.originalFileName;
+                }
 
                 // Look for the attachment added before this attachment in new array.
                 for (attachment in attachmentArray)
                 {
-                    if ([attachment.event.eventId isEqualToString:firstAttachmentEventId])
+                    if (firstAttachmentOriginalFileName && [attachment.originalFileName isEqualToString:firstAttachmentOriginalFileName])
+                    {
+                        break;
+                    }
+                    else if ([attachment.event.eventId isEqualToString:firstAttachmentEventId])
                     {
                         break;
                     }
@@ -313,10 +331,17 @@ - (void)displayAttachments:(NSArray*)attachmentArray focusOn:(NSString*)eventId
             // Compute the attachment index
             NSUInteger currentAttachmentIndex = (isBackPaginationInProgress ? currentVisibleItemIndex - 1 : currentVisibleItemIndex);
 
-            if (currentVisibleItemIndex < attachments.count)
+            if (currentAttachmentIndex < attachments.count)
             {
                 MXKAttachment *attachment = attachments[currentAttachmentIndex];
                 currentAttachmentEventId = attachment.event.eventId;
+
+                // The original file name is used when the attachment is a local echo.
+                // Indeed its event id may be replaced by the actual one in the new attachments array.
+                if ([currentAttachmentEventId hasPrefix:kMXKEventFormatterLocalEventIdPrefix])
+                {
+                    currentAttachmentOriginalFileName = attachment.originalFileName;
+                }
             }
         }
     }
@@ -327,15 +352,23 @@ - (void)displayAttachments:(NSArray*)attachmentArray focusOn:(NSString*)eventId
     // Set/reset the attachments array
     attachments = [NSMutableArray arrayWithArray:attachmentArray];
 
-    // Update the index of the current displayed attachment according to the new attachments array
+    // Update the index of the current displayed attachment by looking for the
+    // current event id (or the current original file name, if any) in the new attachments array.
     currentVisibleItemIndex = 0;
     if (currentAttachmentEventId)
     {
         for (NSUInteger index = 0; index < attachments.count; index++)
         {
             MXKAttachment *attachment = attachments[index];
 
-            if ([attachment.event.eventId isEqualToString:currentAttachmentEventId])
+            // Check first the original filename if any.
+            if (currentAttachmentOriginalFileName && [attachment.originalFileName isEqualToString:currentAttachmentOriginalFileName])
+            {
+                currentVisibleItemIndex = index;
+                break;
+            }
+            // Check the event id then
+            else if ([attachment.event.eventId isEqualToString:currentAttachmentEventId])
             {
                 currentVisibleItemIndex = index;
                 break;
@@ -372,7 +405,11 @@ - (IBAction)hideNavigationBar
 
 - (void)refreshCurrentVisibleItemIndex
 {
-    currentVisibleItemIndex = _attachmentsCollection.contentOffset.x / [[UIScreen mainScreen] bounds].size.width;
+    // Check whether the collection is actually rendered
+    if (_attachmentsCollection.contentSize.width)
+    {
+        currentVisibleItemIndex = _attachmentsCollection.contentOffset.x / [[UIScreen mainScreen] bounds].size.width;
+    }
 }
 
 - (void)refreshAttachmentCollectionContentOffset

diff --git a/MatrixKit/Models/Room/MXKAttachment.h b/MatrixKit/Models/Room/MXKAttachment.h
@@ -54,6 +54,11 @@ typedef enum : NSUInteger {
 @property (nonatomic) NSString *thumbnailURL;
 @property (nonatomic) NSDictionary *thumbnailInfo;
 
+/**
+ The original file name retrieved from the event body (if any).
+ */
+@property (nonatomic, readonly) NSString *originalFileName;
+
 /**
  The actual attachment url
  */

diff --git a/MatrixKit/Models/Room/MXKAttachment.m b/MatrixKit/Models/Room/MXKAttachment.m
@@ -27,11 +27,6 @@ @interface MXKAttachment ()
     id onAttachmentDownloadEndObs;
     id onAttachmentDownloadFailureObs;
 
-    /**
-     The original file name is available in event body (if any).
-     */
-    NSString *originalFileName;
-
     /**
      The local path used to store the attachment with its original name
      */
@@ -82,7 +77,7 @@ - (instancetype)initWithEvent:(MXEvent *)mxEvent andMatrixSession:(MXSession*)mx
             return nil;
         }
 
-        originalFileName = [_event.content[@"body"] isKindOfClass:[NSString class]] ? _event.content[@"body"] : nil;
+        _originalFileName = [_event.content[@"body"] isKindOfClass:[NSString class]] ? _event.content[@"body"] : nil;
     }
     return self;
 }
@@ -269,11 +264,11 @@ - (void)prepareShare:(void (^)(NSURL *fileURL))onReadyToShare failure:(void (^)(
         NSURL *fileUrl;
 
         // Check whether the original name retrieved from event body has extension
-        if (originalFileName && [originalFileName pathExtension].length)
+        if (_originalFileName && [_originalFileName pathExtension].length)
         {
             // Copy the cached file to restore its original name
             // Note:  We used previously symbolic link (instead of copy) but UIDocumentInteractionController failed to open Office documents (.docx, .pptx...).
-            documentCopyPath = [[MXKMediaManager getCachePath] stringByAppendingPathComponent:originalFileName];
+            documentCopyPath = [[MXKMediaManager getCachePath] stringByAppendingPathComponent:_originalFileName];
 
             [[NSFileManager defaultManager] removeItemAtPath:documentCopyPath error:nil];
             if ([[NSFileManager defaultManager] copyItemAtPath:_cacheFilePath toPath:documentCopyPath error:nil])

diff --git a/MatrixKit/Models/Room/MXKRoomDataSource.m b/MatrixKit/Models/Room/MXKRoomDataSource.m
@@ -2404,8 +2404,7 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N
 }
 
 
-#pragma mark - Local echo suppression
-// @TODO: All these dirty methods will be removed once CS v2 is available.
+#pragma mark - Local echo handling
 
 /**
  Add a local echo event waiting for the true event coming down from the event stream.