From 6b6f4f933684d5cd43548f17876e56f253ecc331 Mon Sep 17 00:00:00 2001 From: Michael Kirk <michael@signal.org> Date: Tue, 10 Apr 2018 12:59:26 -0400 Subject: [PATCH 1/2] Limit caption length // FREEBIE --- .../ConversationViewController.m | 3 ++ .../AttachmentApprovalViewController.swift | 40 ++++++++++++++----- .../Messages/Interactions/TSOutgoingMessage.m | 15 ++++++- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 10e4e19b0..e8cdac495 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -4165,6 +4165,9 @@ typedef enum : NSUInteger { DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithOversizeText:text]; SignalAttachment *attachment = [SignalAttachment attachmentWithDataSource:dataSource dataUTI:kOversizeTextAttachmentUTI]; + // TODO we should redundantly send the first n chars in the body field so it can be viewed + // on clients that don't support oversized text messgaes, (and potentially generate a preview + // before the attachment is downloaded) message = [ThreadUtil sendMessageWithAttachment:attachment inThread:self.thread quotedReplyModel:self.inputToolbar.quotedReply diff --git a/SignalMessaging/attachments/AttachmentApprovalViewController.swift b/SignalMessaging/attachments/AttachmentApprovalViewController.swift index db7598bf6..d673e0556 100644 --- a/SignalMessaging/attachments/AttachmentApprovalViewController.swift +++ b/SignalMessaging/attachments/AttachmentApprovalViewController.swift @@ -609,19 +609,26 @@ class CaptioningToolbar: UIView, UITextViewDelegate { // MARK: - UITextViewDelegate public func textViewDidChange(_ textView: UITextView) { - // compute new height assuming width is unchanged - let currentSize = textView.frame.size - let newHeight = clampedTextViewHeight(fixedWidth: currentSize.width) - - if newHeight != self.textViewHeight { - Logger.debug("\(self.logTag) TextView height changed: \(self.textViewHeight) -> \(newHeight)") - self.textViewHeight = newHeight - self.textViewHeightConstraint?.constant = textViewHeight - self.invalidateIntrinsicContentSize() - } + updateHeight(textView: textView) } public func textView(_ textView: UITextView, shouldChangeTextIn range: NSRange, replacementText text: String) -> Bool { + + // Limit caption character count. We do this in characters, not bytes. + // This character limit will be safely below our byte limit (16k) for almost all uses. + // Because the captioning interface doesn't allow newlines, in practice design pressures users to leave relatively short captions. + let maxCharacterCount = 2000 + guard textView.text.count + text.count - range.length <= maxCharacterCount else { + // Accept as much of the input as we can + let remainingSpace = maxCharacterCount - textView.text.count + if (remainingSpace) > 0 { + let acceptableAddition = text.substring(to: text.startIndex.advanced(by: remainingSpace)) + textView.text = "\(textView.text ?? "")\(acceptableAddition)" + updateHeight(textView: textView) + } + return false + } + // Though we can wrap the text, we don't want to encourage multline captions, plus a "done" button // allows the user to get the keyboard out of the way while in the attachment approval view. if text == "\n" { @@ -642,6 +649,19 @@ class CaptioningToolbar: UIView, UITextViewDelegate { // MARK: - Helpers + private func updateHeight(textView: UITextView) { + // compute new height assuming width is unchanged + let currentSize = textView.frame.size + let newHeight = clampedTextViewHeight(fixedWidth: currentSize.width) + + if newHeight != self.textViewHeight { + Logger.debug("\(self.logTag) TextView height changed: \(self.textViewHeight) -> \(newHeight)") + self.textViewHeight = newHeight + self.textViewHeightConstraint?.constant = textViewHeight + self.invalidateIntrinsicContentSize() + } + } + private func clampedTextViewHeight(fixedWidth: CGFloat) -> CGFloat { let contentSize = textView.sizeThatFits(CGSize(width: fixedWidth, height: CGFloat.greatestFiniteMagnitude)) return Clamp(contentSize.height, kMinTextViewHeight, maxTextViewHeight) diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 9189ecd33..129c7ae2c 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -4,6 +4,7 @@ #import "TSOutgoingMessage.h" #import "NSDate+OWS.h" +#import "OWSMessageSender.h" #import "OWSOutgoingSyncMessage.h" #import "OWSSignalServiceProtos.pb.h" #import "ProtoBuf+OWS.h" @@ -415,7 +416,19 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSSignalServiceProtosDataMessageBuilder *builder = [OWSSignalServiceProtosDataMessageBuilder new]; [builder setTimestamp:self.timestamp]; - [builder setBody:self.body]; + + + if ([self.body lengthOfBytesUsingEncoding:NSUTF8StringEncoding] <= kOversizeTextMessageSizeThreshold) { + [builder setBody:self.body]; + } else { + OWSFail(@"%@ message body length too long.", self.logTag); + NSMutableString *truncatedBody = [self.body mutableCopy]; + while ([truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding] > kOversizeTextMessageSizeThreshold) { + DDLogError(@"%@ truncating body which is too long: %tu", self.logTag, [truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding]); + truncatedBody = [truncatedBody substringToIndex:truncatedBody.length / 2]; + } + [builder setBody:truncatedBody]; + } [builder setExpireTimer:self.expiresInSeconds]; // Group Messages From d94709e13f72ad90fdb93637da7bb80d8b7f096a Mon Sep 17 00:00:00 2001 From: Michael Kirk <michael@signal.org> Date: Tue, 10 Apr 2018 13:28:07 -0400 Subject: [PATCH 2/2] Show label when captioning limit has been reached. // FREEBIE --- .../translations/en.lproj/Localizable.strings | 3 +++ .../AttachmentApprovalViewController.swift | 22 +++++++++++++++++++ .../Messages/Interactions/TSOutgoingMessage.m | 8 ++++--- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index ac512867d..e3735813e 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -82,6 +82,9 @@ /* No comment provided by engineer. */ "ATTACHMENT" = "Attachment"; +/* One line label indicating the user can add no more text to the attachment caption. */ +"ATTACHMENT_APPROVAL_CAPTION_LENGTH_LIMIT_REACHED" = "Message limit reached."; + /* Title for the 'attachment approval' dialog. */ "ATTACHMENT_APPROVAL_DIALOG_TITLE" = "Attachment"; diff --git a/SignalMessaging/attachments/AttachmentApprovalViewController.swift b/SignalMessaging/attachments/AttachmentApprovalViewController.swift index d673e0556..db24d8c87 100644 --- a/SignalMessaging/attachments/AttachmentApprovalViewController.swift +++ b/SignalMessaging/attachments/AttachmentApprovalViewController.swift @@ -484,6 +484,7 @@ class CaptioningToolbar: UIView, UITextViewDelegate { private let sendButton: UIButton private let textView: UITextView private let bottomGradient: GradientView + private let lengthLimitLabel: UILabel // Layout Constants @@ -523,6 +524,7 @@ class CaptioningToolbar: UIView, UITextViewDelegate { self.bottomGradient = GradientView(from: UIColor.clear, to: UIColor.black) self.textView = MessageTextView() self.textViewHeight = kMinTextViewHeight + self.lengthLimitLabel = UILabel() super.init(frame: CGRect.zero) @@ -560,12 +562,24 @@ class CaptioningToolbar: UIView, UITextViewDelegate { // Increase hit area of send button sendButton.contentEdgeInsets = UIEdgeInsets(top: 6, left: 8, bottom: 6, right: 8) + // Length Limit Label shown when the user inputs too long of a message + lengthLimitLabel.textColor = .white + lengthLimitLabel.text = NSLocalizedString("ATTACHMENT_APPROVAL_CAPTION_LENGTH_LIMIT_REACHED", comment: "One line label indicating the user can add no more text to the attachment caption.") + lengthLimitLabel.textAlignment = .center + + // Add shadow in case overlayed on white content + lengthLimitLabel.layer.shadowColor = UIColor.black.cgColor + lengthLimitLabel.layer.shadowOffset = CGSize(width: 0.0, height: 0.0) + lengthLimitLabel.layer.shadowOpacity = 0.8 + self.lengthLimitLabel.isHidden = true + let contentView = UIView() addSubview(contentView) contentView.autoPinEdgesToSuperviewEdges() contentView.addSubview(bottomGradient) contentView.addSubview(sendButton) contentView.addSubview(textView) + contentView.addSubview(lengthLimitLabel) // Layout let kToolbarMargin: CGFloat = 8 @@ -597,6 +611,12 @@ class CaptioningToolbar: UIView, UITextViewDelegate { sendButton.setContentHuggingHigh() sendButton.setCompressionResistanceHigh() + lengthLimitLabel.autoPinEdge(toSuperviewMargin: .left) + lengthLimitLabel.autoPinEdge(toSuperviewMargin: .right) + lengthLimitLabel.autoPinEdge(.bottom, to: .top, of: textView, withOffset: -6) + lengthLimitLabel.setContentHuggingHigh() + lengthLimitLabel.setCompressionResistanceHigh() + let bottomGradientHeight = ScaleFromIPhone5(100) bottomGradient.autoSetDimension(.height, toSize: bottomGradientHeight) bottomGradient.autoPinEdgesToSuperviewEdges(with: .zero, excludingEdge: .top) @@ -619,6 +639,7 @@ class CaptioningToolbar: UIView, UITextViewDelegate { // Because the captioning interface doesn't allow newlines, in practice design pressures users to leave relatively short captions. let maxCharacterCount = 2000 guard textView.text.count + text.count - range.length <= maxCharacterCount else { + self.lengthLimitLabel.isHidden = false // Accept as much of the input as we can let remainingSpace = maxCharacterCount - textView.text.count if (remainingSpace) > 0 { @@ -628,6 +649,7 @@ class CaptioningToolbar: UIView, UITextViewDelegate { } return false } + self.lengthLimitLabel.isHidden = true // Though we can wrap the text, we don't want to encourage multline captions, plus a "done" button // allows the user to get the keyboard out of the way while in the attachment approval view. diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 129c7ae2c..b4838f128 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -416,15 +416,17 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSSignalServiceProtosDataMessageBuilder *builder = [OWSSignalServiceProtosDataMessageBuilder new]; [builder setTimestamp:self.timestamp]; - - + + if ([self.body lengthOfBytesUsingEncoding:NSUTF8StringEncoding] <= kOversizeTextMessageSizeThreshold) { [builder setBody:self.body]; } else { OWSFail(@"%@ message body length too long.", self.logTag); NSMutableString *truncatedBody = [self.body mutableCopy]; while ([truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding] > kOversizeTextMessageSizeThreshold) { - DDLogError(@"%@ truncating body which is too long: %tu", self.logTag, [truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding]); + DDLogError(@"%@ truncating body which is too long: %tu", + self.logTag, + [truncatedBody lengthOfBytesUsingEncoding:NSUTF8StringEncoding]); truncatedBody = [truncatedBody substringToIndex:truncatedBody.length / 2]; } [builder setBody:truncatedBody];