render attachments with captions

// FREEBIE

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m

@@ -20,6 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
 @interface BubbleMaskingView : UIView
 
 @property (nonatomic) BOOL isOutgoing;
+@property (nonatomic) BOOL hasCaption;
 @property (nonatomic, nullable, weak) UIView *maskedSubview;
 
 @end
@@ -60,7 +61,17 @@ NS_ASSUME_NONNULL_BEGIN
     // The JSQ masks are not RTL-safe, so we need to invert the
     // mask orientation manually.
     BOOL hasOutgoingMask = self.isOutgoing ^ self.isRTL;
-    [JSQMessagesMediaViewBubbleImageMasker applyBubbleImageMaskToMediaView:maskedSubview isOutgoing:hasOutgoingMask];
+
+    // Since the caption has it's own tail, the media bubble looks better
+    // without a tail.
+    if (self.hasCaption) {
+        self.layoutMargins = UIEdgeInsetsMake(0, 0, 2, 8);
+        maskedSubview.clipsToBounds = YES;
+        maskedSubview.layer.cornerRadius = 10;
+    } else {
+        [JSQMessagesMediaViewBubbleImageMasker applyBubbleImageMaskToMediaView:maskedSubview
+                                                                    isOutgoing:hasOutgoingMask];
+    }
 }
 
 @end
@@ -978,7 +989,7 @@ NS_ASSUME_NONNULL_BEGIN
     // FIXME why disable? make sure we can interact with both media and caption
     //    view.userInteractionEnabled = NO;
     [self.mediaMaskingView addSubview:view];
-    [self.contentConstraints addObjectsFromArray:[view autoPinToSuperviewEdges]];
+    [self.contentConstraints addObjectsFromArray:[view autoPinEdgesToSuperviewMargins]];
     [self cropMediaViewToBubbbleShape:view];
     if (self.isMediaBeingSent) {
         view.layer.opacity = 0.75f;
@@ -1014,6 +1025,7 @@ NS_ASSUME_NONNULL_BEGIN
     OWSAssert(view.superview == self.mediaMaskingView);
 
     self.mediaMaskingView.isOutgoing = self.isOutgoing;
+    self.mediaMaskingView.hasCaption = self.viewItem.hasText;
     self.mediaMaskingView.maskedSubview = view;
     [self.mediaMaskingView updateMask];
 }
@@ -1240,7 +1252,9 @@ NS_ASSUME_NONNULL_BEGIN
     self.myBubbleImageView.hidden = YES;
     //    self.payloadView.maskedSubview = nil;
     self.mediaMaskingView.maskedSubview = nil;
+    self.mediaMaskingView.hasCaption = NO;
     //    self.textMaskingView.maskedSubview = nil;
+    self.mediaMaskingView.layoutMargins = UIEdgeInsetsZero;
 
     [self.stillImageView removeFromSuperview];
     self.stillImageView = nil;

Signal/src/ViewControllers/DebugUI/DebugUIMessages.m

@@ -378,7 +378,12 @@ NS_ASSUME_NONNULL_BEGIN
     NSString *utiType = [MIMETypeUtil utiTypeForFileExtension:filename.pathExtension];
     DataSource *_Nullable dataSource = [DataSourcePath dataSourceWithFilePath:filePath];
     [dataSource setSourceFilename:filename];
-    SignalAttachment *attachment = [SignalAttachment attachmentWithDataSource:dataSource dataUTI:utiType];
+    SignalAttachment *attachment =
+        [SignalAttachment attachmentWithDataSource:dataSource dataUTI:utiType imageQuality:TSImageQualityOriginal];
+    if (arc4random_uniform(100) > 50) {
+        attachment.captionText = [self randomCaptionText];
+    }
+
     OWSAssert(attachment);
     if ([attachment hasError]) {
         DDLogError(@"attachment[%@]: %@", [attachment sourceFilename], [attachment errorName]);
@@ -663,6 +668,11 @@ NS_ASSUME_NONNULL_BEGIN
     [self sendRandomAttachment:thread uti:uti length:256];
 }
 
++ (NSString *)randomCaptionText
+{
+    return [NSString stringWithFormat:@"%@ (caption)", [self randomText]];
+}
+
 + (void)sendRandomAttachment:(TSThread *)thread uti:(NSString *)uti length:(NSUInteger)length
 {
     OWSMessageSender *messageSender = [Environment current].messageSender;
@@ -672,9 +682,9 @@ NS_ASSUME_NONNULL_BEGIN
         [SignalAttachment attachmentWithDataSource:dataSource dataUTI:uti imageQuality:TSImageQualityOriginal];
 
     if (arc4random_uniform(100) > 50) {
-        // give 1/2 our attachments captions, and add a hint that it's a caption since we style them indistinguishably
+        // give 1/2 our attachments captions, and add a hint that it's a caption since we
-        // from a separate text message.
+        // style them indistinguishably from a separate text message.
-        attachment.captionText = [NSString stringWithFormat:@"%@ (caption)", [self randomText]];
+        attachment.captionText = [self randomCaptionText];
     }
     [ThreadUtil sendMessageWithAttachment:attachment inThread:thread messageSender:messageSender ignoreErrors:YES];
 }

SignalServiceKit/src/Messages/Interactions/TSMessage.m

@@ -12,7 +12,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-static const NSUInteger OWSMessageSchemaVersion = 3;
+static const NSUInteger OWSMessageSchemaVersion = 4;
 
 @interface TSMessage ()
 
@@ -127,16 +127,38 @@ static const NSUInteger OWSMessageSchemaVersion = 3;
         return self;
     }
 
+    if (_schemaVersion < 2) {
+        // renamed _attachments to _attachmentIds
+        if (!_attachmentIds) {
+            _attachmentIds = [coder decodeObjectForKey:@"attachments"];
+        }
+    }
+
     if (_schemaVersion < 3) {
         _expiresInSeconds = 0;
         _expireStartedAt = 0;
         _expiresAt = 0;
     }
 
-    if (_schemaVersion < 2) {
+    if (_schemaVersion < 4) {
-        // renamed _attachments to _attachmentIds
+        // Wipe out the body field on these legacy attachment messages.
-        if (!_attachmentIds) {
+        //
-            _attachmentIds = [coder decodeObjectForKey:@"attachments"];
+        // Explantion: Historically, a message sent from iOS could be an attachment XOR a text message,
+        // but now we support sending an attachment+caption as a single message.
+        //
+        // Other clients have supported sending attachment+caption in a single message for a long time.
+        // So the way we used to handle receiving them was to make it look like they'd sent two messages:
+        // first the attachment+caption (we'd ignore this caption when rendering), followed by a separate
+        // message with just the caption (which we'd render as a simple independent text message), for
+        // which we'd offset the timestamp by a little bit to get the desired ordering.
+        //
+        // Now that we can properly render an attachment+caption message together, these legacy "dummy" text
+        // messages are not only unnecessary, but worse, would be rendered redundantly. For safety, rather
+        // than building the logic to try to find and delete the redundant "dummy" text messages which users
+        // have been seeing and interacting with, we delete the body field from the attachment message,
+        // which iOS users has never seen directly.
+        if (_attachmentIds.count > 0) {
+            _body = nil;
         }
     }
 

SignalServiceKit/src/Messages/OWSMessageManager.m

@@ -1054,21 +1054,21 @@ NS_ASSUME_NONNULL_BEGIN
 
     DDLogDebug(@"%@ shouldMarkMessageAsRead: %d (%@)", self.logTag, shouldMarkMessageAsRead, envelope.source);
 
-    // Other clients allow attachments to be sent along with body, we want the text displayed as a separate
+    //    // Other clients allow attachments to be sent along with body, we want the text displayed as a separate
-    // message
+    //    // message
-    if ([attachmentIds count] > 0 && body != nil && body.length > 0) {
+    //    if ([attachmentIds count] > 0 && body != nil && body.length > 0) {
-        // We want the text to be displayed under the attachment.
+    //        // We want the text to be displayed under the attachment.
-        uint64_t textMessageTimestamp = timestamp + 1;
+    //        uint64_t textMessageTimestamp = timestamp + 1;
-        TSIncomingMessage *textMessage = [[TSIncomingMessage alloc] initWithTimestamp:textMessageTimestamp
+    //        TSIncomingMessage *textMessage = [[TSIncomingMessage alloc] initWithTimestamp:textMessageTimestamp
-                                                                             inThread:thread
+    //                                                                             inThread:thread
-                                                                             authorId:envelope.source
+    //                                                                             authorId:envelope.source
-                                                                       sourceDeviceId:envelope.sourceDevice
+    //                                                                       sourceDeviceId:envelope.sourceDevice
-                                                                          messageBody:body
+    //                                                                          messageBody:body
-                                                                        attachmentIds:@[]
+    //                                                                        attachmentIds:@[]
-                                                                     expiresInSeconds:dataMessage.expireTimer];
+    //                                                                     expiresInSeconds:dataMessage.expireTimer];
-        DDLogDebug(@"%@ incoming extra text message: %@", self.logTag, incomingMessage.debugDescription);
+    //        DDLogDebug(@"%@ incoming extra text message: %@", self.logTag, incomingMessage.debugDescription);
-        [textMessage saveWithTransaction:transaction];
+    //        [textMessage saveWithTransaction:transaction];
-    }
+    //    }
 
     // In case we already have a read receipt for this new message (this happens sometimes).
     [OWSReadReceiptManager.sharedManager applyEarlyReadReceiptsForIncomingMessage:incomingMessage