From 19bb6519edf9f2d460b10acd278ab36ec520ce1f Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 3 Jun 2016 09:39:34 -0700 Subject: [PATCH] Fix crash truncating a string of a node with a zero size The crash happens in the placeholderImage of ASTextNode. The node is not visible in the time it try to get the `placeholderImage` and so the `visibleRange` has count 0 and a crash happens while accessing the first object of an empty array. --- AsyncDisplayKit/ASTextNode.mm | 17 +++++++---- .../TextKit/ASTextKitRenderer+TextChecking.mm | 2 +- AsyncDisplayKit/TextKit/ASTextKitRenderer.h | 12 +++++++- AsyncDisplayKit/TextKit/ASTextKitRenderer.mm | 14 +++++++++ .../TextKit/ASTextKitTailTruncater.mm | 11 ++++++- AsyncDisplayKit/TextKit/ASTextKitTruncating.h | 21 ++++++++++--- .../ASTextKitTruncationTests.mm | 30 +++++++++++++++++-- AsyncDisplayKitTests/ASTextNodeTests.m | 9 ++++++ 8 files changed, 100 insertions(+), 16 deletions(-) diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 83f6aad67e..785fdb0dc1 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -469,7 +469,7 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; forHighlighting:(BOOL)highlighting { ASTextKitRenderer *renderer = [self _renderer]; - NSRange visibleRange = renderer.visibleRanges[0]; + NSRange visibleRange = renderer.firstVisibleRange; NSAttributedString *attributedString = _attributedText; NSRange clampedRange = NSIntersectionRange(visibleRange, NSMakeRange(0, attributedString.length)); @@ -784,14 +784,18 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; // FIXME: Replace this implementation with reusable CALayers that have .backgroundColor set. // This would completely eliminate the memory and performance cost of the backing store. CGSize size = self.calculatedSize; + if (CGSizeEqualToSize(size, CGSizeZero)) { + return nil; + } + UIGraphicsBeginImageContext(size); [self.placeholderColor setFill]; ASTextKitRenderer *renderer = [self _renderer]; - NSRange textRange = renderer.visibleRanges[0]; + NSRange visibleRange = renderer.firstVisibleRange; // cap height is both faster and creates less subpixel blending - NSArray *lineRects = [self _rectsForTextRange:textRange measureOption:ASTextKitRendererMeasureOptionLineHeight]; + NSArray *lineRects = [self _rectsForTextRange:visibleRange measureOption:ASTextKitRendererMeasureOptionLineHeight]; // fill each line with the placeholder color for (NSValue *rectValue in lineRects) { @@ -860,7 +864,8 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; BOOL linkCrossesVisibleRange = (lastCharIndex > range.location) && (lastCharIndex < NSMaxRange(range) - 1); if (inAdditionalTruncationMessage) { - NSRange visibleRange = [self _renderer].visibleRanges[0]; + ASTextKitRenderer *renderer = [self _renderer]; + NSRange visibleRange = renderer.firstVisibleRange; NSRange truncationMessageRange = [self _additionalTruncationMessageRangeWithVisibleRange:visibleRange]; [self _setHighlightRange:truncationMessageRange forAttributeName:ASTextNodeTruncationTokenAttributeName value:nil animated:YES]; } else if (range.length && !linkCrossesVisibleRange && linkAttributeValue != nil && linkAttributeName != nil) { @@ -1054,8 +1059,8 @@ static NSAttributedString *DefaultTruncationAttributedString() - (BOOL)isTruncated { - NSRange visibleRange = [self _renderer].visibleRanges[0]; - return visibleRange.length < _attributedText.length; + ASTextKitRenderer *renderer = [self _renderer]; + return renderer.firstVisibleRange.length < _attributedText.length; } - (void)setPointSizeScaleFactors:(NSArray *)pointSizeScaleFactors diff --git a/AsyncDisplayKit/TextKit/ASTextKitRenderer+TextChecking.mm b/AsyncDisplayKit/TextKit/ASTextKitRenderer+TextChecking.mm index bb9bd653b5..f9e56f4e0d 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitRenderer+TextChecking.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitRenderer+TextChecking.mm @@ -59,7 +59,6 @@ NSAttributedString *truncationAttributedString = self.attributes.truncationAttributedString; // get the index of the last character, so we can handle text in the truncation token - NSRange visibleRange = self.truncater.visibleRanges[0]; __block NSRange truncationTokenRange = { NSNotFound, 0 }; [truncationAttributedString enumerateAttribute:ASTextKitTruncationAttributeName inRange:NSMakeRange(0, truncationAttributedString.length) @@ -75,6 +74,7 @@ truncationTokenRange = { 0, truncationAttributedString.length }; } + NSRange visibleRange = self.truncater.firstVisibleRange; truncationTokenRange.location += NSMaxRange(visibleRange); __block CGFloat minDistance = CGFLOAT_MAX; diff --git a/AsyncDisplayKit/TextKit/ASTextKitRenderer.h b/AsyncDisplayKit/TextKit/ASTextKitRenderer.h index e57c5dc174..577482eef5 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitRenderer.h +++ b/AsyncDisplayKit/TextKit/ASTextKitRenderer.h @@ -78,7 +78,7 @@ The character range from the original attributedString that is displayed by the renderer given the parameters in the initializer. */ -- (std::vector)visibleRanges; +@property (nonatomic, assign, readonly) std::vector visibleRanges; /** The number of lines shown in the string. @@ -86,3 +86,13 @@ - (NSUInteger)lineCount; @end + +@interface ASTextKitRenderer (ASTextKitRendererConvenience) + +/** + Returns the first visible range or an NSRange with location of NSNotFound and size of 0 if no first visible + range exists + */ +@property (nonatomic, assign, readonly) NSRange firstVisibleRange; + +@end diff --git a/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm b/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm index 15f0ecd5fa..b4161a4965 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm @@ -262,3 +262,17 @@ static NSCharacterSet *_defaultAvoidTruncationCharacterSet() } @end + +@implementation ASTextKitRenderer (ASTextKitRendererConvenience) + +- (NSRange)firstVisibleRange +{ + std::vector visibleRanges = self.visibleRanges; + if (visibleRanges.size() > 0) { + return visibleRanges[0]; + } + + return NSMakeRange(NSNotFound, 0); +} + +@end diff --git a/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm b/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm index 7f77c3d76a..916dc69530 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm @@ -20,7 +20,6 @@ NSCharacterSet *_avoidTailTruncationSet; } @synthesize visibleRanges = _visibleRanges; -@synthesize truncationStringRect = _truncationStringRect; - (instancetype)initWithContext:(ASTextKitContext *)context truncationAttributedString:(NSAttributedString *)truncationAttributedString @@ -185,4 +184,14 @@ }]; } +- (NSRange)firstVisibleRange +{ + std::vector visibleRanges = _visibleRanges; + if (visibleRanges.size() > 0) { + return visibleRanges[0]; + } + + return NSMakeRange(NSNotFound, 0); +} + @end diff --git a/AsyncDisplayKit/TextKit/ASTextKitTruncating.h b/AsyncDisplayKit/TextKit/ASTextKitTruncating.h index 91f79c0740..70b4799b5a 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitTruncating.h +++ b/AsyncDisplayKit/TextKit/ASTextKitTruncating.h @@ -14,10 +14,21 @@ #import "ASTextKitRenderer.h" +NS_ASSUME_NONNULL_BEGIN + @protocol ASTextKitTruncating +/** + The character range from the original attributedString that is displayed by the renderer given the parameters in the + initializer. + */ @property (nonatomic, assign, readonly) std::vector visibleRanges; -@property (nonatomic, assign, readonly) CGRect truncationStringRect; + +/** + Returns the first visible range or an NSRange with location of NSNotFound and size of 0 if no first visible + range exists + */ +@property (nonatomic, assign, readonly) NSRange firstVisibleRange; /** A truncater object is initialized with the full state of the text. It is a Single Responsibility Object that is @@ -30,12 +41,14 @@ The truncater should not store a strong reference to the context to prevent retain cycles. */ - (instancetype)initWithContext:(ASTextKitContext *)context - truncationAttributedString:(NSAttributedString *)truncationAttributedString - avoidTailTruncationSet:(NSCharacterSet *)avoidTailTruncationSet; + truncationAttributedString:(NSAttributedString * _Nullable)truncationAttributedString + avoidTailTruncationSet:(NSCharacterSet * _Nullable)avoidTailTruncationSet; /** - * Actually do the truncation. + Actually do the truncation. */ - (void)truncate; @end + +NS_ASSUME_NONNULL_END diff --git a/AsyncDisplayKitTests/ASTextKitTruncationTests.mm b/AsyncDisplayKitTests/ASTextKitTruncationTests.mm index dab6b240e7..8973c78405 100644 --- a/AsyncDisplayKitTests/ASTextKitTruncationTests.mm +++ b/AsyncDisplayKitTests/ASTextKitTruncationTests.mm @@ -55,6 +55,7 @@ avoidTailTruncationSet:nil]; [tailTruncater truncate]; XCTAssert(NSEqualRanges(textKitVisibleRange, tailTruncater.visibleRanges[0])); + XCTAssert(NSEqualRanges(textKitVisibleRange, tailTruncater.firstVisibleRange)); } - (void)testSimpleTailTruncation @@ -80,6 +81,7 @@ NSString *expectedString = @"90's cray photo booth tote bag bespoke Carles. Plaid wayfarers..."; XCTAssertEqualObjects(expectedString, drawnString); XCTAssert(NSEqualRanges(NSMakeRange(0, 62), tailTruncater.visibleRanges[0])); + XCTAssert(NSEqualRanges(NSMakeRange(0, 62), tailTruncater.firstVisibleRange)); } - (void)testAvoidedCharTailWordBoundaryTruncation @@ -132,6 +134,27 @@ XCTAssertEqualObjects(expectedString, drawnString); } +- (void)testHandleZeroSizeConstrainedSize +{ + CGSize constrainedSize = CGSizeZero; + NSAttributedString *attributedString = [self _sentenceAttributedString]; + + ASTextKitContext *context = [[ASTextKitContext alloc] initWithAttributedString:attributedString + lineBreakMode:NSLineBreakByWordWrapping + maximumNumberOfLines:0 + exclusionPaths:nil + constrainedSize:constrainedSize + layoutManagerCreationBlock:nil + layoutManagerDelegate:nil + textStorageCreationBlock:nil]; + ASTextKitTailTruncater *tailTruncater = [[ASTextKitTailTruncater alloc] initWithContext:context + truncationAttributedString:[self _simpleTruncationAttributedString] + avoidTailTruncationSet:nil]; + XCTAssertNoThrow([tailTruncater truncate]); + XCTAssert(tailTruncater.visibleRanges.size() == 0); + NSEqualRanges(NSMakeRange(NSNotFound, 0), tailTruncater.firstVisibleRange); +} + - (void)testHandleZeroHeightConstrainedSize { CGSize constrainedSize = CGSizeMake(50, 0); @@ -145,9 +168,10 @@ layoutManagerDelegate:nil textStorageCreationBlock:nil]; - XCTAssertNoThrow([[[ASTextKitTailTruncater alloc] initWithContext:context - truncationAttributedString:[self _simpleTruncationAttributedString] - avoidTailTruncationSet:[NSCharacterSet characterSetWithCharactersInString:@"."]] truncate]); + ASTextKitTailTruncater *tailTruncater = [[ASTextKitTailTruncater alloc] initWithContext:context + truncationAttributedString:[self _simpleTruncationAttributedString] + avoidTailTruncationSet:[NSCharacterSet characterSetWithCharactersInString:@"."]]; + XCTAssertNoThrow([tailTruncater truncate]); } @end diff --git a/AsyncDisplayKitTests/ASTextNodeTests.m b/AsyncDisplayKitTests/ASTextNodeTests.m index 6696d7c507..b37bb37314 100644 --- a/AsyncDisplayKitTests/ASTextNodeTests.m +++ b/AsyncDisplayKitTests/ASTextNodeTests.m @@ -140,6 +140,15 @@ static BOOL CGSizeEqualToSizeWithIn(CGSize size1, CGSize size2, CGFloat delta) } } +- (void)testMeasureWithZeroSizeAndPlaceholder +{ + _textNode.placeholderEnabled = YES; + + XCTAssertNoThrow([_textNode measure:CGSizeZero], @"Measure with zero size and placeholder enabled should not throw an exception"); + XCTAssertNoThrow([_textNode measure:CGSizeMake(0, 100)], @"Measure with zero width and placeholder enabled should not throw an exception"); + XCTAssertNoThrow([_textNode measure:CGSizeMake(100, 0)], @"Measure with zero height and placeholder enabled should not throw an exception"); +} + - (void)testAccessibility { _textNode.attributedText = _attributedText;