From bce18637f038b9fe4fb49d0b5648a13317273969 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 19 Dec 2017 10:18:10 -0600 Subject: [PATCH] render attachments with captions // FREEBIE --- .../ConversationView/Cells/OWSMessageCell.m | 18 +++++++++-- .../ViewControllers/DebugUI/DebugUIMessages.m | 18 ++++++++--- .../src/Messages/Interactions/TSMessage.m | 32 ++++++++++++++++--- .../src/Messages/OWSMessageManager.m | 30 ++++++++--------- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m index 5ea8d0d4a..4f1d06398 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m +++ b/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; diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index db80c1a0c..a7b5f7e42 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/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 - // from a separate text message. - attachment.captionText = [NSString stringWithFormat:@"%@ (caption)", [self randomText]]; + // give 1/2 our attachments captions, and add a hint that it's a caption since we + // style them indistinguishably from a separate text message. + attachment.captionText = [self randomCaptionText]; } [ThreadUtil sendMessageWithAttachment:attachment inThread:thread messageSender:messageSender ignoreErrors:YES]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index 8207da26b..1f6fbac51 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/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) { - // renamed _attachments to _attachmentIds - if (!_attachmentIds) { - _attachmentIds = [coder decodeObjectForKey:@"attachments"]; + if (_schemaVersion < 4) { + // Wipe out the body field on these legacy attachment messages. + // + // 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; } } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 9eb7ec568..e66f363df 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/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 - // message - if ([attachmentIds count] > 0 && body != nil && body.length > 0) { - // We want the text to be displayed under the attachment. - uint64_t textMessageTimestamp = timestamp + 1; - TSIncomingMessage *textMessage = [[TSIncomingMessage alloc] initWithTimestamp:textMessageTimestamp - inThread:thread - authorId:envelope.source - sourceDeviceId:envelope.sourceDevice - messageBody:body - attachmentIds:@[] - expiresInSeconds:dataMessage.expireTimer]; - DDLogDebug(@"%@ incoming extra text message: %@", self.logTag, incomingMessage.debugDescription); - [textMessage saveWithTransaction:transaction]; - } + // // Other clients allow attachments to be sent along with body, we want the text displayed as a separate + // // message + // if ([attachmentIds count] > 0 && body != nil && body.length > 0) { + // // We want the text to be displayed under the attachment. + // uint64_t textMessageTimestamp = timestamp + 1; + // TSIncomingMessage *textMessage = [[TSIncomingMessage alloc] initWithTimestamp:textMessageTimestamp + // inThread:thread + // authorId:envelope.source + // sourceDeviceId:envelope.sourceDevice + // messageBody:body + // attachmentIds:@[] + // expiresInSeconds:dataMessage.expireTimer]; + // DDLogDebug(@"%@ incoming extra text message: %@", self.logTag, incomingMessage.debugDescription); + // [textMessage saveWithTransaction:transaction]; + // } // In case we already have a read receipt for this new message (this happens sometimes). [OWSReadReceiptManager.sharedManager applyEarlyReadReceiptsForIncomingMessage:incomingMessage