Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid querying for NSAttributedString attributes on empty string #1614

Merged
merged 1 commit into from
Aug 13, 2019
Merged

Avoid querying for NSAttributedString attributes on empty string #1614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Source/ASTextNode2.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions Source/TextKit/ASTextKitContext.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions Tests/ASTextNode2Tests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions Tests/ASTextNodeTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down