From 4caf9511f4a26deb47fc0f686691a8c3fbb9accd Mon Sep 17 00:00:00 2001 From: Rahul Malik Date: Tue, 13 Aug 2019 14:39:27 -0700 Subject: [PATCH] Avoid querying for NSAttributedString attributes on empty string (#1614) --- Source/ASTextNode2.mm | 5 ++--- Source/TextKit/ASTextKitContext.mm | 6 ++---- Tests/ASTextNode2Tests.mm | 9 +++++++++ Tests/ASTextNodeTests.mm | 9 +++++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index 001ebddd7..796fc0aec 100644 --- 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 index 5ef840c66..dfde01fe4 100644 --- a/Source/TextKit/ASTextKitContext.mm +++ b/Source/TextKit/ASTextKitContext.mm @@ -50,7 +50,6 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString __instanceLock__ = std::make_shared(); // Create the TextKit component stack with our default configuration. - _textStorage = [[NSTextStorage alloc] init]; _layoutManager = [[ASLayoutManager alloc] init]; _layoutManager.usesFontLeading = NO; @@ -58,14 +57,13 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString // 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 index 9f2b8947f..70d25cb26 100644 --- 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 index 77b6954bb..e01f2516f 100644 --- 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 {