Avoid querying for NSAttributedString attributes on empty string (#1614)

TextureGroup · Aug 13, 2019 · 4caf951 · 4caf951
1 parent 0c78681
commit 4caf951
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 7 deletions.
diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm
@@ -519,12 +519,11 @@ - (void)prepareAttributedString:(NSMutableAttributedString *)attributedString is
   }
 
   // Apply tint color if needed and foreground color is not already specified
-  if (self.textColorFollowsTintColor) {
+  if (self.textColorFollowsTintColor && attributedString.length > 0) {
     // Apply tint color if specified and if foreground color is undefined for attributedString
     NSRange limit = NSMakeRange(0, attributedString.length);
-    NSRange effectiveRange;
     // Look for previous attributes that define foreground color
-    UIColor *attributeValue = (UIColor *)[attributedString attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:&effectiveRange];
+    UIColor *attributeValue = (UIColor *)[attributedString attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:NULL];
     UIColor *tintColor = self.tintColor;
     if (attributeValue == nil && tintColor) {
       // None are found, apply tint color if available. Fallback to "black" text color

diff --git a/Source/TextKit/ASTextKitContext.mm b/Source/TextKit/ASTextKitContext.mm
@@ -50,22 +50,20 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString
     __instanceLock__ = std::make_shared<AS::Mutex>();
 
     // Create the TextKit component stack with our default configuration.
-
     _textStorage = [[NSTextStorage alloc] init];
     _layoutManager = [[ASLayoutManager alloc] init];
     _layoutManager.usesFontLeading = NO;
     [_textStorage addLayoutManager:_layoutManager];
 
     // Instead of calling [NSTextStorage initWithAttributedString:], setting attributedString just after calling addlayoutManager can fix CJK language layout issues.
     // See https://github.com/facebook/AsyncDisplayKit/issues/2894
-    if (attributedString) {
+    if (attributedString && attributedString.length > 0) {
       [_textStorage setAttributedString:attributedString];
 
       // Apply tint color if specified and if foreground color is undefined for attributedString
       NSRange limit = NSMakeRange(0, attributedString.length);
-      NSRange effectiveRange;
       // Look for previous attributes that define foreground color
-      UIColor *attributeValue = (UIColor *)[attributedString attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:&effectiveRange];
+      UIColor *attributeValue = (UIColor *)[attributedString attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:NULL];
       if (attributeValue == nil) {
         // None are found, apply tint color if available. Fallback to "black" text color
         if (tintColor) {

diff --git a/Tests/ASTextNode2Tests.mm b/Tests/ASTextNode2Tests.mm
@@ -121,4 +121,13 @@ - (void)testSupportsLayerBacking
   XCTAssertFalse(textNode.supportsLayerBacking);
 }
 
+- (void)testEmptyStringSize
+{
+  CGSize constrainedSize = CGSizeMake(100, CGFLOAT_MAX);
+  _textNode.attributedText = [[NSAttributedString alloc] initWithString:@""];
+  CGSize sizeWithEmptyString = [_textNode layoutThatFits:ASSizeRangeMake(CGSizeZero, constrainedSize)].size;
+  XCTAssertTrue(ASIsCGSizeValidForSize(sizeWithEmptyString));
+  XCTAssertTrue(sizeWithEmptyString.width == 0);
+}
+
 @end
diff --git a/Tests/ASTextNodeTests.mm b/Tests/ASTextNodeTests.mm
@@ -262,6 +262,15 @@ - (void)testAddingExclusionPathsShouldInvalidateAndIncreaseTheSize
   XCTAssertGreaterThan(sizeWithExclusionPaths.height, sizeWithoutExclusionPaths.height, @"Setting exclusions paths should invalidate the calculated size and return a greater size");
 }
 
+- (void)testEmptyStringSize
+{
+  CGSize constrainedSize = CGSizeMake(100, CGFLOAT_MAX);
+  _textNode.attributedText = [[NSAttributedString alloc] initWithString:@""];
+  CGSize sizeWithEmptyString = [_textNode layoutThatFits:ASSizeRangeMake(CGSizeZero, constrainedSize)].size;
+  XCTAssertTrue(ASIsCGSizeValidForSize(sizeWithEmptyString));
+  XCTAssertTrue(sizeWithEmptyString.width == 0);
+}
+
 #if AS_ENABLE_TEXTNODE
 - (void)testThatTheExperimentWorksCorrectly
 {