Merge pull request #1706 from maicki/FixASTextKitRendererCrashIfNoVisibleRange

[ASTextKitRenderer] Fix crash truncating a string of an ASTextNode with a zero size

Author: appleguy

AsyncDisplayKit/ASTextNode.mm

@@ -469,7 +469,7 @@ - (id)_linkAttributeValueAtPoint:(CGPoint)point
                  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 @@ - (UIImage *)placeholderImage
   // 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 @@ - (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event
   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 @@ - (void)setTruncationMode:(NSLineBreakMode)truncationMode
 
 - (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

AsyncDisplayKit/TextKit/ASTextKitRenderer+TextChecking.mm

@@ -59,7 +59,6 @@ - (NSTextCheckingResult *)textCheckingResultAtPoint:(CGPoint)point
   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 @@ - (NSTextCheckingResult *)textCheckingResultAtPoint:(CGPoint)point
     truncationTokenRange = { 0, truncationAttributedString.length };
   }
 
+  NSRange visibleRange = self.truncater.firstVisibleRange;
   truncationTokenRange.location += NSMaxRange(visibleRange);
 
   __block CGFloat minDistance = CGFLOAT_MAX;

AsyncDisplayKit/TextKit/ASTextKitRenderer.h

@@ -78,11 +78,21 @@
  The character range from the original attributedString that is displayed by the renderer given the parameters in the
  initializer.
  */
-- (std::vector<NSRange>)visibleRanges;
+@property (nonatomic, assign, readonly) std::vector<NSRange> visibleRanges;
 
 /**
  The number of lines shown in the string.
  */
 - (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

AsyncDisplayKit/TextKit/ASTextKitRenderer.mm

@@ -262,3 +262,17 @@ - (NSUInteger)lineCount
 }
 
 @end
+
+@implementation ASTextKitRenderer (ASTextKitRendererConvenience)
+
+- (NSRange)firstVisibleRange
+{
+  std::vector<NSRange> visibleRanges = self.visibleRanges;
+  if (visibleRanges.size() > 0) {
+    return visibleRanges[0];
+  }
+  
+  return NSMakeRange(NSNotFound, 0);
+}
+
+@end

AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm

@@ -20,7 +20,6 @@ @implementation ASTextKitTailTruncater
   NSCharacterSet *_avoidTailTruncationSet;
 }
 @synthesize visibleRanges = _visibleRanges;
-@synthesize truncationStringRect = _truncationStringRect;
 
 - (instancetype)initWithContext:(ASTextKitContext *)context
      truncationAttributedString:(NSAttributedString *)truncationAttributedString
@@ -185,4 +184,14 @@ - (void)truncate
   }];
 }
 
+- (NSRange)firstVisibleRange
+{
+  std::vector<NSRange> visibleRanges = _visibleRanges;
+  if (visibleRanges.size() > 0) {
+    return visibleRanges[0];
+  }
+
+  return NSMakeRange(NSNotFound, 0);
+}
+
 @end

AsyncDisplayKit/TextKit/ASTextKitTruncating.h

@@ -14,10 +14,21 @@
 
 #import "ASTextKitRenderer.h"
 
+NS_ASSUME_NONNULL_BEGIN
+
 @protocol ASTextKitTruncating <NSObject>
 
+/**
+ 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<NSRange> 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

AsyncDisplayKitTests/ASTextKitTruncationTests.mm

@@ -55,6 +55,7 @@ - (void)testEmptyTruncationStringSameAsStraightTextKitTailTruncation
                                                                    avoidTailTruncationSet:nil];
   [tailTruncater truncate];
   XCTAssert(NSEqualRanges(textKitVisibleRange, tailTruncater.visibleRanges[0]));
+  XCTAssert(NSEqualRanges(textKitVisibleRange, tailTruncater.firstVisibleRange));
 }
 
 - (void)testSimpleTailTruncation
@@ -80,6 +81,7 @@ - (void)testSimpleTailTruncation
   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 @@ - (void)testAvoidedCharTailCharBoundaryTruncation
   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 @@ - (void)testHandleZeroHeightConstrainedSize
                                                            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

AsyncDisplayKitTests/ASTextNodeTests.m

@@ -140,6 +140,15 @@ - (void)testRecalculationOfSizeIsSameAsOriginallyCalculatedFloatingPointSize
   }
 }
 
+- (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;