From 979386ee9eb34518b40ef04a7753175eac74bea9 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 17 Jan 2018 13:39:51 -0500 Subject: [PATCH] Improve handling of text and url shares. --- .../AttachmentApprovalViewController.swift | 8 +-- .../attachments/MediaMessageView.swift | 9 +-- .../SharingThreadPickerViewController.m | 72 +++++++++++-------- .../attachments/SignalAttachment.swift | 7 +- SignalMessaging/categories/UIView+OWS.h | 4 ++ SignalMessaging/categories/UIView+OWS.m | 33 +++++++++ .../ShareViewController.swift | 24 ++++--- 7 files changed, 109 insertions(+), 48 deletions(-) diff --git a/SignalMessaging/attachments/AttachmentApprovalViewController.swift b/SignalMessaging/attachments/AttachmentApprovalViewController.swift index abd0c9f5e..f2d80fc40 100644 --- a/SignalMessaging/attachments/AttachmentApprovalViewController.swift +++ b/SignalMessaging/attachments/AttachmentApprovalViewController.swift @@ -130,8 +130,8 @@ public class AttachmentApprovalViewController: OWSViewController, CaptioningTool scrollView.autoPinEdgesToSuperviewEdges() let defaultCaption = self.defaultCaption() - let isUrlShare = defaultCaption != nil - let backgroundColor = isUrlShare ? UIColor.ows_signalBrandBlue : UIColor.black + let isTextualShare = defaultCaption != nil + let backgroundColor = isTextualShare ? UIColor.ows_signalBrandBlue : UIColor.black self.view.backgroundColor = backgroundColor // Create full screen container view so the scrollView @@ -237,12 +237,12 @@ public class AttachmentApprovalViewController: OWSViewController, CaptioningTool } private func defaultCaption() -> String? { - guard self.attachment.isUrl else { + guard self.attachment.isUrl || self.attachment.isText else { return nil } let data = self.attachment.data guard let messageText = String(data: data, encoding: String.Encoding.utf8) else { - Logger.error("\(self.logTag) Couldn't load url strubg") + Logger.error("\(self.logTag) Couldn't load url or text string") return nil } return messageText diff --git a/SignalMessaging/attachments/MediaMessageView.swift b/SignalMessaging/attachments/MediaMessageView.swift index 48440b358..348df54a5 100644 --- a/SignalMessaging/attachments/MediaMessageView.swift +++ b/SignalMessaging/attachments/MediaMessageView.swift @@ -110,10 +110,11 @@ public class MediaMessageView: UIView, OWSAudioAttachmentPlayerDelegate { createVideoPreview() } else if attachment.isAudio { createAudioPreview() + // We handle the isOversizeText case before isText. } else if attachment.isOversizeText { + createOversizeTextPreview() + } else if attachment.isUrl || attachment.isText { createTextPreview() - } else if attachment.isUrl { - createUrlPreview() } else { createGenericPreview() } @@ -290,7 +291,7 @@ public class MediaMessageView: UIView, OWSAudioAttachmentPlayerDelegate { } } - private func createTextPreview() { + private func createOversizeTextPreview() { let data = attachment.data guard let messageText = String(data: data, encoding: String.Encoding.utf8) else { @@ -342,7 +343,7 @@ public class MediaMessageView: UIView, OWSAudioAttachmentPlayerDelegate { messageTextView.autoPinTrailingToSuperview(withMargin:15) } - private func createUrlPreview() { + private func createTextPreview() { // Show nothing; URLs should only appear in the attachment approval view // of the SAE and in this context the URL will be placed in the caption field. } diff --git a/SignalMessaging/attachments/SharingThreadPickerViewController.m b/SignalMessaging/attachments/SharingThreadPickerViewController.m index 0fb3eff60..f1bf3d4e3 100644 --- a/SignalMessaging/attachments/SharingThreadPickerViewController.m +++ b/SignalMessaging/attachments/SharingThreadPickerViewController.m @@ -18,6 +18,8 @@ NS_ASSUME_NONNULL_BEGIN +typedef void (^SendCompletionBlock)(NSError *_Nullable, TSOutgoingMessage *); + @interface SharingThreadPickerViewController () @@ -193,8 +195,7 @@ NS_ASSUME_NONNULL_BEGIN } #endif - void (^sendCompletion)(NSError *_Nullable, TSOutgoingMessage *) = ^( - NSError *_Nullable error, TSOutgoingMessage *message) { + SendCompletionBlock sendCompletion = ^(NSError *_Nullable error, TSOutgoingMessage *message) { dispatch_async(dispatch_get_main_queue(), ^{ if (error) { @@ -215,33 +216,46 @@ NS_ASSUME_NONNULL_BEGIN }); }; - [fromViewController - presentViewController:progressAlert - animated:YES - completion:^(void) { - __block TSOutgoingMessage *outgoingMessage = nil; - if (self.attachment.isUrl && self.attachment.captionText.length > 0) { - // Urls are added to the caption text, so discard the attachment - // and send the caption as a regular text message. - NSString *messageText = self.attachment.captionText; - outgoingMessage = [ThreadUtil sendMessageWithText:messageText - inThread:self.thread - messageSender:self.messageSender - success:^{ - sendCompletion(nil, outgoingMessage); - } - failure:^(NSError *_Nonnull error) { - sendCompletion(error, outgoingMessage); - }]; - } else { - outgoingMessage = [ThreadUtil sendMessageWithAttachment:self.attachment - inThread:self.thread - messageSender:self.messageSender - completion:^(NSError *_Nullable error) { - sendCompletion(error, outgoingMessage); - }]; - } - }]; + [fromViewController presentViewController:progressAlert + animated:YES + completion:^(void) { + if ((self.attachment.isUrl || self.attachment.isText) + && self.attachment.captionText.length > 0) { + // Urls and text shares are added to the caption text, so discard + // the attachment and send the caption as a regular text message. + NSString *messageText = self.attachment.captionText; + [self sendAsTextMessage:messageText sendCompletion:sendCompletion]; + } else { + [self sendAsAttachmentMessage:sendCompletion]; + } + }]; +} + +- (void)sendAsTextMessage:(NSString *)messageText sendCompletion:(SendCompletionBlock)sendCompletion +{ + OWSAssert(messageText.length > 0); + + __block TSOutgoingMessage *outgoingMessage = nil; + outgoingMessage = [ThreadUtil sendMessageWithText:messageText + inThread:self.thread + messageSender:self.messageSender + success:^{ + sendCompletion(nil, outgoingMessage); + } + failure:^(NSError *_Nonnull error) { + sendCompletion(error, outgoingMessage); + }]; +} + +- (void)sendAsAttachmentMessage:(SendCompletionBlock)sendCompletion +{ + __block TSOutgoingMessage *outgoingMessage = nil; + outgoingMessage = [ThreadUtil sendMessageWithAttachment:self.attachment + inThread:self.thread + messageSender:self.messageSender + completion:^(NSError *_Nullable error) { + sendCompletion(error, outgoingMessage); + }]; } - (void)showSendFailureAlertWithError:(NSError *)error diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 253a67cf4..ea259f750 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -419,9 +419,14 @@ public class SignalAttachment: NSObject { return dataUTI == kOversizeTextAttachmentUTI } + @objc + public var isText: Bool { + return UTTypeConformsTo(dataUTI as CFString, kUTTypeText) || isOversizeText + } + @objc public var isUrl: Bool { - return dataUTI == (kUTTypeURL as String) + return UTTypeConformsTo(dataUTI as CFString, kUTTypeURL) } @objc diff --git a/SignalMessaging/categories/UIView+OWS.h b/SignalMessaging/categories/UIView+OWS.h index d7eb6359a..d299c3b5d 100644 --- a/SignalMessaging/categories/UIView+OWS.h +++ b/SignalMessaging/categories/UIView+OWS.h @@ -117,6 +117,10 @@ CGFloat ScaleFromIPhone5(CGFloat iPhone5Value); // Add red border to self, and all subviews recursively. - (void)addRedBorderRecursively; +- (void)logFrameLater; +- (void)logFrameLaterWithLabel:(NSString *)label; +- (void)logHierarchyUpwardLaterWithLabel:(NSString *)label; + @end NS_ASSUME_NONNULL_END diff --git a/SignalMessaging/categories/UIView+OWS.m b/SignalMessaging/categories/UIView+OWS.m index f34469268..9512b96f3 100644 --- a/SignalMessaging/categories/UIView+OWS.m +++ b/SignalMessaging/categories/UIView+OWS.m @@ -496,6 +496,39 @@ CGFloat ScaleFromIPhone5(CGFloat iPhone5Value) } } +- (void)logFrameLater +{ + [self logFrameLaterWithLabel:@""]; +} + +- (void)logFrameLaterWithLabel:(NSString *)label +{ + dispatch_async(dispatch_get_main_queue(), ^{ + DDLogVerbose(@"%@ %@ frame: %@, hidden: %d, opacity: %f", + self.logTag, + label, + NSStringFromCGRect(self.frame), + self.hidden, + self.layer.opacity); + }); +} + +- (void)logHierarchyUpwardLaterWithLabel:(NSString *)label +{ + dispatch_async(dispatch_get_main_queue(), ^{ + DDLogVerbose(@"%@ %@ ----", self.logTag, label); + }); + + UIResponder *responder = self; + while (responder) { + if ([responder isKindOfClass:[UIView class]]) { + UIView *view = (UIView *)responder; + [view logFrameLaterWithLabel:@"\t"]; + } + responder = responder.nextResponder; + } +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index f5d9d5107..1cfa286ed 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -538,17 +538,17 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } Logger.info("\(self.logTag) attachment: \(itemProvider)") - guard let utiType = ShareViewController.utiTypeForItem(itemProvider: itemProvider) else { + guard let srcUtiType = ShareViewController.utiTypeForItem(itemProvider: itemProvider) else { let error = ShareViewControllerError.unsupportedMedia return Promise(error: error) } - Logger.debug("\(logTag) matched utiType: \(utiType)") + Logger.debug("\(logTag) matched utiType: \(srcUtiType)") - let (promise, fulfill, reject) = Promise.pending() + let (promise, fulfill, reject) = Promise<(URL, String)>.pending() var customFileName: String? - itemProvider.loadItem(forTypeIdentifier: utiType, options: nil, completionHandler: { + itemProvider.loadItem(forTypeIdentifier: srcUtiType, options: nil, completionHandler: { (provider, error) in guard error == nil else { @@ -570,14 +570,14 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE customFileName = "Contact.vcf" } - let customFileExtension = MIMETypeUtil.fileExtension(forUTIType:utiType) + let customFileExtension = MIMETypeUtil.fileExtension(forUTIType:srcUtiType) guard let tempFilePath = OWSFileSystem.writeData(toTemporaryFile: data, fileExtension: customFileExtension) else { let writeError = ShareViewControllerError.assertionError(description: "Error writing item data: \(String(describing: error))") reject(writeError) return } let fileUrl = URL(fileURLWithPath:tempFilePath) - fulfill(fileUrl) + fulfill((fileUrl, srcUtiType)) } else if let string = provider as? String { guard let data = string.data(using: String.Encoding.utf8) else { let writeError = ShareViewControllerError.assertionError(description: "Error writing item data: \(String(describing: error))") @@ -590,9 +590,13 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return } let fileUrl = URL(fileURLWithPath:tempFilePath) - fulfill(fileUrl) + if UTTypeConformsTo(srcUtiType as CFString, kUTTypeText) { + fulfill((fileUrl, srcUtiType)) + } else { + fulfill((fileUrl, kUTTypeText as String)) + } } else if let url = provider as? URL { - fulfill(url) + fulfill((url, srcUtiType)) } else { let unexpectedTypeError = ShareViewControllerError.assertionError(description: "unexpected item type: \(String(describing: provider))") reject(unexpectedTypeError) @@ -602,7 +606,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // TODO accept other data types // TODO whitelist attachment types // TODO coerce when necessary and possible - return promise.then { (itemUrl: URL) -> Promise in + return promise.then { (itemUrl: URL, utiType: String) -> Promise in let url: URL = try { if self.isVideoNeedingRelocation(itemProvider: itemProvider, itemUrl: itemUrl) { @@ -612,7 +616,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } }() - Logger.debug("\(self.logTag) building DataSource with url: \(url)") + Logger.debug("\(self.logTag) building DataSource with url: \(url), utiType: \(utiType)") guard let dataSource = ShareViewController.createDataSource(utiType : utiType, url : url, customFileName : customFileName) else { throw ShareViewControllerError.assertionError(description: "Unable to read attachment data")