keejef
/
session-ios
mirror of https://github.com/oxen-io/session-ios
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix cropped Chinese/Japanese messages

Browse Source 
The earlier fix for the broken ios10 emoji font ended up breaking
messages for some users with a tall font.

Here we have a lighter touch - ensuring we don't touch messages that
don't use emoji.

Also, introduce a different approach to the fix, rather than trying to
compute the bounding rect of an appropriately attributed string, just
add an extra bit of height per line.

This approach isn't ideal for long messages with only one emoji line in
them, but the previous approach was incompatible with Chinese messages
that also contain emoji. See the new
`MesssagesBubblesSizeCalculatorTest.swift` for test cases considered.

// FREEBIE
pull/1/head
Michael Kirk 9 years ago
parent 62f9606bf5
commit bd370f1de4
6 changed files with 170 additions and 126 deletions
Show Stats Download Patch File Download Diff File

4
Signal.xcodeproj/project.pbxproj
Unescape Escape View File

@ -18,6 +18,7 @@
451DE9FD1DC1A28200810E42 /* SyncPushTokensJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 451DE9FC1DC1A28200810E42 /* SyncPushTokensJob.swift */; };
451DE9FE1DC1A28200810E42 /* SyncPushTokensJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 451DE9FC1DC1A28200810E42 /* SyncPushTokensJob.swift */; };
4520D8D51D417D8E00123472 /* Photos.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4520D8D41D417D8E00123472 /* Photos.framework */; };
452D1EE81DCA90D100A57EC4 /* MesssagesBubblesSizeCalculatorTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 452D1EE71DCA90D100A57EC4 /* MesssagesBubblesSizeCalculatorTest.swift */; };
452E3C8E1D935C77002A45B0 /* OWSConversationSettingsTableViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 452E3C8D1D935C77002A45B0 /* OWSConversationSettingsTableViewController.m */; };
452E3C8F1D935C77002A45B0 /* OWSConversationSettingsTableViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 452E3C8D1D935C77002A45B0 /* OWSConversationSettingsTableViewController.m */; };
453D28B71D32BA5F00D523F0 /* OWSDisplayedMessage.m in Sources */ = {isa = PBXBuildFile; fileRef = 453D28B61D32BA5F00D523F0 /* OWSDisplayedMessage.m */; };
@ -544,6 +545,7 @@
451DE9FC1DC1A28200810E42 /* SyncPushTokensJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SyncPushTokensJob.swift; sourceTree = "<group>"; };
4520D8D41D417D8E00123472 /* Photos.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Photos.framework; path = System/Library/Frameworks/Photos.framework; sourceTree = SDKROOT; };
4526BD481CA61C8D00166BC8 /* OWSMessageEditing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSMessageEditing.h; sourceTree = "<group>"; };
452D1EE71DCA90D100A57EC4 /* MesssagesBubblesSizeCalculatorTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = MesssagesBubblesSizeCalculatorTest.swift; path = Models/MesssagesBubblesSizeCalculatorTest.swift; sourceTree = "<group>"; };
452E3C8C1D935C77002A45B0 /* OWSConversationSettingsTableViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSConversationSettingsTableViewController.h; sourceTree = "<group>"; };
452E3C8D1D935C77002A45B0 /* OWSConversationSettingsTableViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; lineEnding = 0; path = OWSConversationSettingsTableViewController.m; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objc; };
453CC0361D08E1A60040EBA3 /* sn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sn; path = translations/sn.lproj/Localizable.strings; sourceTree = "<group>"; };
@ -1226,6 +1228,7 @@
children = (
458E38391D6699FA0094BD24 /* OWSDeviceProvisioningURLParserTest.m */,
458967101DC117CC00E9DD21 /* AccountManagerTest.swift */,
452D1EE71DCA90D100A57EC4 /* MesssagesBubblesSizeCalculatorTest.swift */,
);
name = Models;
sourceTree = "<group>";
@ -3037,6 +3040,7 @@
B660F72E1C29988E00687D6E /* HttpManager.m in Sources */,
458E383A1D6699FA0094BD24 /* OWSDeviceProvisioningURLParserTest.m in Sources */,
B660F72F1C29988E00687D6E /* HttpSocket.m in Sources */,
452D1EE81DCA90D100A57EC4 /* MesssagesBubblesSizeCalculatorTest.swift in Sources */,
B660F7301C29988E00687D6E /* IpAddress.m in Sources */,
B660F7311C29988E00687D6E /* IpEndPoint.m in Sources */,
B660F7321C29988E00687D6E /* PacketHandler.m in Sources */,

5
Signal/src/Models/OWSMessagesBubblesSizeCalculator.h
Unescape Escape View File

@ -2,6 +2,11 @@
#import <JSQMessagesViewController/JSQMessagesBubblesSizeCalculator.h>
NS_SWIFT_NAME(MessagesBubblesSizeCalculator)
@interface OWSMessagesBubblesSizeCalculator : JSQMessagesBubblesSizeCalculator
- (CGSize)messageBubbleSizeForMessageData:(id<JSQMessageData>)messageData
atIndexPath:(NSIndexPath *)indexPath
withLayout:(JSQMessagesCollectionViewFlowLayout *)layout;
@end

144
Signal/src/Models/OWSMessagesBubblesSizeCalculator.m
Unescape Escape View File

@ -10,9 +10,10 @@
NS_ASSUME_NONNULL_BEGIN
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
// superclass protected methods we need in order to compute bubble size.
@interface OWSMessagesBubblesSizeCalculator (OWSiOS10EmojiBug)
/**
* We use some private method to size our info messages.
*/
@interface OWSMessagesBubblesSizeCalculator (JSQPrivateMethods)
@property (strong, nonatomic, readonly) NSCache *cache;
@property (assign, nonatomic, readonly) NSUInteger minimumBubbleWidth;
@ -24,7 +25,6 @@ NS_ASSUME_NONNULL_BEGIN
withLayout:(JSQMessagesCollectionViewFlowLayout *)layout;
- (CGFloat)textBubbleWidthForLayout:(JSQMessagesCollectionViewFlowLayout *)layout;
@end
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
@implementation OWSMessagesBubblesSizeCalculator
@ -56,9 +56,7 @@ NS_ASSUME_NONNULL_BEGIN
CGSize size;
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
BOOL isIOS10OrGreater =
[[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 10 }];
if (isIOS10OrGreater) {
if ([self shouldApplyiOS10EmojiFixToString:messageData.text font:layout.messageBubbleFont]) {
size = [self withiOS10EmojiFixSuperMessageBubbleSizeForMessageData:messageData
atIndexPath:indexPath
withLayout:layout];
@ -67,108 +65,60 @@ NS_ASSUME_NONNULL_BEGIN
}
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
return CGSizeMake(size.width, size.height);
}
/**
* Emoji sizing bug only affects iOS10. Unfortunately the "fix" for emoji font breaks some other fonts, so it's
* important
* to only apply it when emoji is actually present.
*/
- (BOOL)shouldApplyiOS10EmojiFixToString:(NSString *)string font:(UIFont *)font
{
BOOL isIOS10OrGreater =
[[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 10 }];
if (!isIOS10OrGreater) {
return NO;
}
return size;
__block BOOL foundEmoji = NO;
NSDictionary *attributes = @{ NSFontAttributeName : font };
NSMutableAttributedString *attributedString =
[[NSMutableAttributedString alloc] initWithString:string attributes:attributes];
[attributedString fixAttributesInRange:NSMakeRange(0, string.length)];
[attributedString enumerateAttribute:NSFontAttributeName
inRange:NSMakeRange(0, string.length)
options:0
usingBlock:^(id _Nullable value, NSRange range, BOOL *_Nonnull stop) {
UIFont *rangeFont = (UIFont *)value;
if ([rangeFont.fontName isEqualToString:@".AppleColorEmojiUI"]) {
DDLogVerbose(@"Detected Emoji at location: %lu, for length: %lu",
(unsigned long)range.location,
(unsigned long)range.length);
foundEmoji = YES;
*stop = YES;
}
}];
return foundEmoji;
}
/**
* HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
* iOS10 bug in rendering emoji requires to fudge some things in the middle of the super method.
* Copy/pasted the superclass method and inlined (and marked) our hacks inline.
* As of iOS10.0 the UIEmoji font doesn't present proper line heights. In some cases this causes the last line in a
* message to get cropped off.
*/
- (CGSize)withiOS10EmojiFixSuperMessageBubbleSizeForMessageData:(id<JSQMessageData>)messageData
atIndexPath:(NSIndexPath *)indexPath
withLayout:(JSQMessagesCollectionViewFlowLayout *)layout
{
NSValue *cachedSize = [self.cache objectForKey:@([messageData messageHash])];
if (cachedSize != nil) {
return [cachedSize CGSizeValue];
}
CGSize finalSize = CGSizeZero;
if ([messageData isMediaMessage]) {
finalSize = [[messageData media] mediaViewDisplaySize];
} else {
CGSize avatarSize = [self jsq_avatarSizeForMessageData:messageData withLayout:layout];
UIFont *emojiFont = [UIFont fontWithName:@".AppleColorEmojiUI" size:layout.messageBubbleFont.pointSize];
CGSize superSize = [super messageBubbleSizeForMessageData:messageData atIndexPath:indexPath withLayout:layout];
int lines = (int)floor(superSize.height / emojiFont.lineHeight);
// from the cell xibs, there is a 2 point space between avatar and bubble
CGFloat spacingBetweenAvatarAndBubble = 2.0f;
CGFloat horizontalContainerInsets = layout.messageBubbleTextViewTextContainerInsets.left
+ layout.messageBubbleTextViewTextContainerInsets.right;
CGFloat horizontalFrameInsets
= layout.messageBubbleTextViewFrameInsets.left + layout.messageBubbleTextViewFrameInsets.right;
CGFloat horizontalInsetsTotal
= horizontalContainerInsets + horizontalFrameInsets + spacingBetweenAvatarAndBubble;
CGFloat maximumTextWidth = [self textBubbleWidthForLayout:layout] - avatarSize.width
- layout.messageBubbleLeftRightMargin - horizontalInsetsTotal;
///////////////////
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
// //stringRect doesn't give the correct size with the new emoji font.
// CGRect stringRect = [[messageData text] boundingRectWithSize:CGSizeMake(maximumTextWidth, CGFLOAT_MAX)
// options:(NSStringDrawingUsesLineFragmentOrigin |
// NSStringDrawingUsesFontLeading)
// attributes:@{ NSFontAttributeName :
// layout.messageBubbleFont }
// context:nil];
CGRect stringRect;
if (!messageData.text) {
stringRect = CGRectZero;
} else {
NSDictionary *attributes = @{ NSFontAttributeName : layout.messageBubbleFont };
NSMutableAttributedString *string =
[[NSMutableAttributedString alloc] initWithString:[messageData text] attributes:attributes];
[string fixAttributesInRange:NSMakeRange(0, string.length)];
[string
enumerateAttribute:NSFontAttributeName
inRange:NSMakeRange(0, string.length)
options:0
usingBlock:^(id _Nullable value, NSRange range, BOOL *_Nonnull stop) {
UIFont *font = (UIFont *)value;
if ([font.fontName isEqualToString:@".AppleColorEmojiUI"]) {
DDLogVerbose(@"Replacing new broken emoji font with old emoji font at location: %lu, "
@"for length: %lu",
(unsigned long)range.location,
(unsigned long)range.length);
[string addAttribute:NSFontAttributeName
value:[UIFont fontWithName:@"AppleColorEmoji" size:font.pointSize]
range:range];
}
}];
stringRect =
[string boundingRectWithSize:CGSizeMake(maximumTextWidth, CGFLOAT_MAX)
options:(NSStringDrawingUsesLineFragmentOrigin | NSStringDrawingUsesFontLeading)
context:nil];
}
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
/////////////////////////
CGSize stringSize = CGRectIntegral(stringRect).size;
CGFloat verticalContainerInsets = layout.messageBubbleTextViewTextContainerInsets.top
+ layout.messageBubbleTextViewTextContainerInsets.bottom;
CGFloat verticalFrameInsets
= layout.messageBubbleTextViewFrameInsets.top + layout.messageBubbleTextViewFrameInsets.bottom;
// add extra 2 points of space (`self.additionalInset`), because `boundingRectWithSize:` is slightly off
// not sure why. magix. (shrug) if you know, submit a PR
CGFloat verticalInsets = verticalContainerInsets + verticalFrameInsets + self.additionalInset;
// same as above, an extra 2 points of magix
CGFloat finalWidth
= MAX(stringSize.width + horizontalInsetsTotal, self.minimumBubbleWidth) + self.additionalInset;
finalSize = CGSizeMake(finalWidth, stringSize.height + verticalInsets);
}
[self.cache setObject:[NSValue valueWithCGSize:finalSize] forKey:@([messageData messageHash])];
return finalSize;
// Add an extra pixel per line to fit the emoji.
return CGSizeMake(superSize.width, superSize.height + lines);
}

29
Signal/src/view controllers/MessagesViewController.m
Unescape Escape View File

@ -794,9 +794,6 @@ typedef enum : NSUInteger {
{
JSQMessagesCollectionViewCell *cell =
(JSQMessagesCollectionViewCell *)[super collectionView:self.collectionView cellForItemAtIndexPath:indexPath];
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
[self fixupiOS10EmojiBugForTextView:cell.textView];
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
if (!message.isMediaMessage) {
cell.textView.textColor = [UIColor ows_blackColor];
cell.textView.linkTextAttributes = @{
@ -815,10 +812,6 @@ typedef enum : NSUInteger {
= (OWSOutgoingMessageCollectionViewCell *)[super collectionView:self.collectionView
cellForItemAtIndexPath:indexPath];
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
[self fixupiOS10EmojiBugForTextView:cell.textView];
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
if (message.isMediaMessage) {
if (![message isKindOfClass:[TSMessageAdapter class]]) {
DDLogError(@"%@ Unexpected media message:%@", self.tag, message.class);
@ -835,28 +828,6 @@ typedef enum : NSUInteger {
return cell;
}
// BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
- (void)fixupiOS10EmojiBugForTextView:(UITextView *)textView
{
BOOL isIOS10OrGreater =
[[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 10 }];
if (isIOS10OrGreater) {
[textView.textStorage enumerateAttribute:NSFontAttributeName
inRange:NSMakeRange(0, textView.textStorage.length)
options:0
usingBlock:^(id _Nullable value, NSRange range, BOOL *_Nonnull stop) {
UIFont *font = (UIFont *)value;
if ([font.fontName isEqualToString:@".AppleColorEmojiUI"]) {
[textView.textStorage addAttribute:NSFontAttributeName
value:[UIFont fontWithName:@"AppleColorEmoji"
size:font.pointSize]
range:range];
}
}];
}
}
// END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368
- (OWSCallCollectionViewCell *)loadCallCellForCall:(OWSCall *)call atIndexPath:(NSIndexPath *)indexPath
{
OWSCallCollectionViewCell *callCell = [self.collectionView dequeueReusableCellWithReuseIdentifier:[OWSCallCollectionViewCell cellReuseIdentifier]

111
Signal/test/Models/MesssagesBubblesSizeCalculatorTest.swift
Unescape Escape View File

@ -0,0 +1,111 @@
// Created by Michael Kirk on 11/2/16.
// Copyright © 2016 Open Whisper Systems. All rights reserved.
import XCTest
class FakeMessageData: NSObject, JSQMessageData {
public func senderId() -> String! {
return "fake-sender-id"
}
func senderDisplayName() -> String! {
return "fake-senderDisplayName"
}
func date() -> Date! {
return Date()
}
@objc func isMediaMessage() -> Bool {
return false
}
@objc func messageHash() -> UInt {
return 1
}
var bodyText: String = "fake message data text"
func text() -> String {
return self.bodyText
}
init(text: String) {
self.bodyText = text;
}
}
class FakeiPhone6JSQMessagesCollectionViewFlowLayout: JSQMessagesCollectionViewFlowLayout {
// This value was nabbed by inspecting the super class layout.itemSize while debugging the `messageBubbleSizeForMessageData`.
// It requires the view to actually be rendered to get a proper size, so we're baking it in here.
// This will break if we change the layout.
override var itemWidth: CGFloat { return 367 }
}
/**
* This is a brittle test, which will break if our layout changes. It serves mostly as documentation for cases to
* consider when changing the bubble size calculator. Primarly these test cases came out of a bug introduced in iOS10,
* which prevents us from computing proper boudning box for text that uses the UIEmoji font.
*
* If one of these tests breaks, it should be OK to update the expected value so long as you've tested the result renders
* correctly in the running app (the reference sizes ewre computed in the context of an iphone6 layour.
* @see `FakeiPhone6JSQMessagesCollectionViewFlowLayout`
*/
class MesssagesBubblesSizeCalculatorTest: XCTestCase {
let indexPath = IndexPath()
let layout = FakeiPhone6JSQMessagesCollectionViewFlowLayout()
let calculator = MessagesBubblesSizeCalculator()
func testHeightForShort1LineMessage() {
let messageData = FakeMessageData(text:"foo")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
XCTAssertEqual(38, actual.height);
}
func testHeightForLong1LineMessage() {
let messageData = FakeMessageData(text:"1 2 3 4 5 6 7 8 9 10 11 12 13 14 x")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
XCTAssertEqual(38, actual.height);
}
func testHeightForShort2LineMessage() {
let messageData = FakeMessageData(text:"1 2 3 4 5 6 7 8 9 10 11 12 13 14 x 1")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
XCTAssertEqual(59, actual.height);
}
func testHeightForLong2LineMessage() {
let messageData = FakeMessageData(text:"1 2 3 4 5 6 7 8 9 10 11 12 13 14 x 1 2 3 4 5 6 7 8 9 10 11 12 13 14 x")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
XCTAssertEqual(59, actual.height);
}
func testHeightForiOS10EmojiBug() {
let messageData = FakeMessageData(text:"Wunderschönen Guten Morgaaaahhhn 😝 - hast du gut geschlafen ☺️😘")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
XCTAssertEqual(84, actual.height);
}
func testHeightForChineseWithEmojiBug() {
let messageData = FakeMessageData(text:"一二三四五六七八九十甲乙丙😝戊己庚辛壬圭咖啡牛奶餅乾水果蛋糕")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
// erroneously seeing 69 with the emoji fix in place.
XCTAssertEqual(84, actual.height);
}
func testHeightForChineseWithoutEmojiBug() {
let messageData = FakeMessageData(text:"一二三四五六七八九十甲乙丙丁戊己庚辛壬圭咖啡牛奶餅乾水果蛋糕")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
// erroneously seeing 69 with the emoji fix in place.
XCTAssertEqual(81, actual.height);
}
func testHeightForiOS10DoubleSpaceNumbersBug() {
let messageData = FakeMessageData(text:"１２３４５６７８９０１２３４５６７８９０")
let actual = calculator.messageBubbleSize(for: messageData, at: indexPath, with: layout)
// erroneously seeing 51 with emoji fix in place. It's the call to "fix string"
XCTAssertEqual(59, actual.height);
}
}

3
Signal/test/SignalTests-Bridging-Header.h
Unescape Escape View File

@ -2,3 +2,6 @@
// Use this file to import your target's public headers that you would like to expose to Swift.
//
#import "Signal-Bridging-Header.h"
#import "OWSMessagesBubblesSizeCalculator.h"
#import <JSQMessagesViewController/JSQMessageData.h>
#import <JSQMessagesViewController/JSQMessagesCollectionViewFlowLayout.h>

Write Preview
Loading…
Cancel
Save