Merge branch 'mkirk/link-preview-restrictions'

pull/2/head
Michael Kirk 6 years ago
commit 8237a43d80

@ -1,5 +1,5 @@
// //
// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // Copyright (c) 2019 Open Whisper Systems. All rights reserved.
// //
#import <SignalMessaging/OWSTextView.h> #import <SignalMessaging/OWSTextView.h>
@ -23,6 +23,7 @@ NS_ASSUME_NONNULL_BEGIN
@protocol ConversationTextViewToolbarDelegate <NSObject> @protocol ConversationTextViewToolbarDelegate <NSObject>
- (void)textViewDidChange:(UITextView *)textView; - (void)textViewDidChange:(UITextView *)textView;
- (void)textViewDidChangeSelection:(UITextView *)textView;
@end @end

@ -189,6 +189,11 @@ NS_ASSUME_NONNULL_BEGIN
[self.textViewToolbarDelegate textViewDidChange:self]; [self.textViewToolbarDelegate textViewDidChange:self];
} }
- (void)textViewDidChangeSelection:(UITextView *)textView
{
[self.textViewToolbarDelegate textViewDidChangeSelection:self];
}
#pragma mark - Key Commands #pragma mark - Key Commands
- (nullable NSArray<UIKeyCommand *> *)keyCommands - (nullable NSArray<UIKeyCommand *> *)keyCommands

@ -869,6 +869,11 @@ const CGFloat kMaxTextViewHeight = 98;
[self updateInputLinkPreview]; [self updateInputLinkPreview];
} }
- (void)textViewDidChangeSelection:(UITextView *)textView
{
[self updateInputLinkPreview];
}
- (void)updateHeightWithTextView:(UITextView *)textView - (void)updateHeightWithTextView:(UITextView *)textView
{ {
// compute new height assuming width is unchanged // compute new height assuming width is unchanged
@ -922,7 +927,10 @@ const CGFloat kMaxTextViewHeight = 98;
return; return;
} }
NSString *_Nullable previewUrl = [OWSLinkPreview previewUrlForMessageBodyText:body]; // It's key that we use the *raw/unstripped* text, so we can reconcile cursor position with the
// selectedRange.
NSString *_Nullable previewUrl = [OWSLinkPreview previewUrlForRawBodyText:self.inputTextView.text
selectedRange:self.inputTextView.selectedRange];
if (previewUrl.length < 1) { if (previewUrl.length < 1) {
[self clearLinkPreviewStateAndView]; [self clearLinkPreviewStateAndView];
return; return;

@ -1,5 +1,5 @@
// //
// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // Copyright (c) 2019 Open Whisper Systems. All rights reserved.
// //
#import "SignalBaseTest.h" #import "SignalBaseTest.h"
@ -63,6 +63,10 @@ NS_ASSUME_NONNULL_BEGIN
return nil; return nil;
} }
- (nonnull NSString *)displayNameForPhoneIdentifier:(NSString * _Nullable)recipientId transaction:(nonnull YapDatabaseReadTransaction *)transaction {
return nil;
}
@end @end
#pragma mark - #pragma mark -

@ -429,10 +429,14 @@ public class OWSLinkPreview: MTLModel {
// MARK: - Text Parsing // MARK: - Text Parsing
// This cache should only be accessed on main thread. // This cache should only be accessed on main thread.
private static var previewUrlCache: NSCache<AnyObject, AnyObject> = NSCache() private static var previewUrlCache: NSCache<NSString, NSString> = NSCache()
@objc @objc
public class func previewUrl(forMessageBodyText body: String?) -> String? { public class func previewUrl(forRawBodyText body: String?, selectedRange: NSRange) -> String? {
return previewUrl(forMessageBodyText: body, selectedRange: selectedRange)
}
public class func previewUrl(forMessageBodyText body: String?, selectedRange: NSRange?) -> String? {
AssertIsOnMainThread() AssertIsOnMainThread()
// Exit early if link previews are not enabled in order to avoid // Exit early if link previews are not enabled in order to avoid
@ -440,38 +444,62 @@ public class OWSLinkPreview: MTLModel {
guard OWSLinkPreview.featureEnabled else { guard OWSLinkPreview.featureEnabled else {
return nil return nil
} }
guard SSKPreferences.areLinkPreviewsEnabled() else { guard SSKPreferences.areLinkPreviewsEnabled() else {
return nil return nil
} }
if let cachedUrl = previewUrlCache.object(forKey: body as AnyObject) as? String { guard let body = body else {
return nil
}
if let cachedUrl = previewUrlCache.object(forKey: body as NSString) as String? {
Logger.verbose("URL parsing cache hit.") Logger.verbose("URL parsing cache hit.")
guard cachedUrl.count > 0 else { guard cachedUrl.count > 0 else {
return nil return nil
} }
return cachedUrl return cachedUrl
} }
let previewUrls = allPreviewUrls(forMessageBodyText: body) let previewUrlMatches = allPreviewUrlMatches(forMessageBodyText: body)
guard let previewUrl = previewUrls.first else { guard let urlMatch = previewUrlMatches.first else {
// Use empty string to indicate "no preview URL" in the cache. // Use empty string to indicate "no preview URL" in the cache.
previewUrlCache.setObject("" as AnyObject, forKey: body as AnyObject) previewUrlCache.setObject("", forKey: body as NSString)
return nil return nil
} }
previewUrlCache.setObject(previewUrl as AnyObject, forKey: body as AnyObject) if let selectedRange = selectedRange {
return previewUrl Logger.verbose("match: urlString: \(urlMatch.urlString) range: \(urlMatch.matchRange) selectedRange: \(selectedRange)")
let cursorAtEndOfMatch = urlMatch.matchRange.location + urlMatch.matchRange.length == selectedRange.location
if selectedRange.location != body.count,
(urlMatch.matchRange.intersection(selectedRange) != nil || cursorAtEndOfMatch) {
Logger.debug("ignoring URL, since the user is currently editing it.")
// we don't want to cache the result here, as we want to fetch the link preview
// if the user moves the cursor.
return nil
}
Logger.debug("considering URL, since the user is not currently editing it.")
}
previewUrlCache.setObject(urlMatch.urlString as NSString, forKey: body as NSString)
return urlMatch.urlString
} }
class func allPreviewUrls(forMessageBodyText body: String?) -> [String] { struct URLMatchResult {
let urlString: String
let matchRange: NSRange
}
class func allPreviewUrls(forMessageBodyText body: String) -> [String] {
return allPreviewUrlMatches(forMessageBodyText: body).map { $0.urlString }
}
class func allPreviewUrlMatches(forMessageBodyText body: String) -> [URLMatchResult] {
guard OWSLinkPreview.featureEnabled else { guard OWSLinkPreview.featureEnabled else {
return [] return []
} }
guard SSKPreferences.areLinkPreviewsEnabled() else { guard SSKPreferences.areLinkPreviewsEnabled() else {
return [] return []
} }
guard let body = body else {
return []
}
let detector: NSDataDetector let detector: NSDataDetector
do { do {
@ -481,7 +509,7 @@ public class OWSLinkPreview: MTLModel {
return [] return []
} }
var previewUrls = [String]() var urlMatches: [URLMatchResult] = []
let matches = detector.matches(in: body, options: [], range: NSRange(location: 0, length: body.count)) let matches = detector.matches(in: body, options: [], range: NSRange(location: 0, length: body.count))
for match in matches { for match in matches {
guard let matchURL = match.url else { guard let matchURL = match.url else {
@ -490,21 +518,22 @@ public class OWSLinkPreview: MTLModel {
} }
let urlString = matchURL.absoluteString let urlString = matchURL.absoluteString
if isValidLinkUrl(urlString) { if isValidLinkUrl(urlString) {
previewUrls.append(urlString) let matchResult = URLMatchResult(urlString: urlString, matchRange: match.range)
urlMatches.append(matchResult)
} }
} }
return previewUrls return urlMatches
} }
// MARK: - Preview Construction // MARK: - Preview Construction
// This cache should only be accessed on serialQueue. // This cache should only be accessed on serialQueue.
private static var linkPreviewDraftCache: NSCache<AnyObject, OWSLinkPreviewDraft> = NSCache() private static var linkPreviewDraftCache: NSCache<NSString, OWSLinkPreviewDraft> = NSCache()
private class func cachedLinkPreview(forPreviewUrl previewUrl: String) -> OWSLinkPreviewDraft? { private class func cachedLinkPreview(forPreviewUrl previewUrl: String) -> OWSLinkPreviewDraft? {
var result: OWSLinkPreviewDraft? var result: OWSLinkPreviewDraft?
serialQueue.sync { serialQueue.sync {
result = linkPreviewDraftCache.object(forKey: previewUrl as AnyObject) result = linkPreviewDraftCache.object(forKey: previewUrl as NSString)
} }
return result return result
} }
@ -522,7 +551,7 @@ public class OWSLinkPreview: MTLModel {
} }
serialQueue.sync { serialQueue.sync {
previewUrlCache.setObject(linkPreviewDraft, forKey: previewUrl as AnyObject) linkPreviewDraftCache.setObject(linkPreviewDraft, forKey: previewUrl as NSString)
} }
} }

@ -169,19 +169,20 @@ class OWSLinkPreviewTest: SSKBaseTestSwift {
} }
func testPreviewUrlForMessageBodyText() { func testPreviewUrlForMessageBodyText() {
XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "")) Assert(bodyText: "", extractsLink: nil)
XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim")) Assert(bodyText: "alice bob jim", extractsLink: nil)
XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://")) Assert(bodyText: "alice bob jim http://", extractsLink: nil)
XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://a.com")) Assert(bodyText: "alice bob jim http://a.com", extractsLink: nil)
XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "https://www.youtube.com/watch?v=tP-Ipsat90c"), Assert(bodyText: "https://www.youtube.com/watch?v=tP-Ipsat90c",
"https://www.youtube.com/watch?v=tP-Ipsat90c") extractsLink: "https://www.youtube.com/watch?v=tP-Ipsat90c")
XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim"),
"https://www.youtube.com/watch?v=tP-Ipsat90c") Assert(bodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim",
extractsLink: "https://www.youtube.com/watch?v=tP-Ipsat90c")
// If there are more than one, take the first. // If there are more than one, take the first.
XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim https://www.youtube.com/watch?v=other-url carol"), Assert(bodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim https://www.youtube.com/watch?v=other-url carol",
"https://www.youtube.com/watch?v=tP-Ipsat90c") extractsLink: "https://www.youtube.com/watch?v=tP-Ipsat90c")
} }
func testUtils() { func testUtils() {
@ -454,4 +455,87 @@ class OWSLinkPreviewTest: SSKBaseTestSwift {
options: [], options: [],
range: NSRange(location: 0, length: text.utf16.count))) range: NSRange(location: 0, length: text.utf16.count)))
} }
func testCursorPositions() {
// sanity check
Assert(bodyText: "https://www.youtube.com/watch?v=testCursorPositionsa",
extractsLink: "https://www.youtube.com/watch?v=testCursorPositionsa",
selectedRange: nil)
// Don't extract link if cursor is touching text
let text2 = "https://www.youtube.com/watch?v=testCursorPositionsb"
XCTAssertEqual(text2.count, 52)
Assert(bodyText: text2,
extractsLink: nil,
selectedRange: NSRange(location: 51, length: 0))
Assert(bodyText: text2,
extractsLink: nil,
selectedRange: NSRange(location: 51, length: 10))
Assert(bodyText: text2,
extractsLink: nil,
selectedRange: NSRange(location: 0, length: 0))
// Unless the cursor is at the end of the text
Assert(bodyText: text2,
extractsLink: "https://www.youtube.com/watch?v=testCursorPositionsb",
selectedRange: NSRange(location: 52, length: 0))
// Once extracted, keep the existing link preview, even if the cursor moves back.
Assert(bodyText: text2,
extractsLink: "https://www.youtube.com/watch?v=testCursorPositionsb",
selectedRange: NSRange(location: 51, length: 0))
let text3 = "foo https://www.youtube.com/watch?v=testCursorPositionsc bar"
XCTAssertEqual(text3.count, 60)
// front edge
Assert(bodyText: text3,
extractsLink: nil,
selectedRange: NSRange(location: 4, length: 0))
// middle
Assert(bodyText: text3,
extractsLink: nil,
selectedRange: NSRange(location: 4, length: 0))
// rear edge
Assert(bodyText: text3,
extractsLink: nil,
selectedRange: NSRange(location: 56, length: 0))
// extract link if selecting after link
Assert(bodyText: text3,
extractsLink: "https://www.youtube.com/watch?v=testCursorPositionsc",
selectedRange: NSRange(location: 57, length: 0))
let text4 = "bar https://www.youtube.com/watch?v=testCursorPositionsd foo"
XCTAssertEqual(text4.count, 60)
// front edge
Assert(bodyText: text4,
extractsLink: nil,
selectedRange: NSRange(location: 4, length: 0))
// middle
Assert(bodyText: text4,
extractsLink: nil,
selectedRange: NSRange(location: 20, length: 0))
// rear edge
Assert(bodyText: text4,
extractsLink: nil,
selectedRange: NSRange(location: 56, length: 0))
// extract link if selecting before link
Assert(bodyText: text4,
extractsLink: "https://www.youtube.com/watch?v=testCursorPositionsd",
selectedRange: NSRange(location: 3, length: 0))
}
private func Assert(bodyText: String, extractsLink link: String?, selectedRange: NSRange? = nil, file: StaticString = #file, line: UInt = #line) {
let actual = OWSLinkPreview.previewUrl(forMessageBodyText: bodyText, selectedRange: selectedRange)
XCTAssertEqual(actual, link, file: file, line: line)
}
} }

Loading…
Cancel
Save