[ASTextNode] Incorrect maximum number of lines with line spacing

[maicki]

ASTextNode uses Text Kit internally to calculate the amount to shrink needed to result in the specified maximum number of lines. Unfortunately, in certain cases using a custom value for lineSpacing and maximumNumberOfLines with ASTextNode can result in the wrong number of lines.

Reproduction Case:

1. Create ASTextNode with maximumNumberOfLines of 3
2. Set attributedText with lineSpacing > 0 (won't repro if = 0)
3. Set truncation string with the same attributes
   --> At last we would get a wrong number of lines.

We have a specific section with a workaround now in the ASTextNode docs:

Incorrect maximum number of lines with line spacing

Although there exists a workaround we should still investigate to get a full picture what is going on behind the scenes.

This issue replaces (and all coming in in the future around this topic):

• [ASTextNode] Truncation issue in text with lineSpacing #140
• [ASButtonNode / ASTextNode] Incorrect maximum number of lines with line spacing #200
• [ASTextNode] Truncation issue in text with lineSpacing facebookarchive/AsyncDisplayKit#2105
• [ASButtonNode / ASTextNode] Incorrect maximum number of lines with line spacing facebookarchive/AsyncDisplayKit#3128
• set the lineSpacing of the ASTextNode seems not correct? facebookarchive/AsyncDisplayKit#3174

[appleguy]

Very interesting - excellent clarifying articulation of all of the factors and other references to this!

By some coincidence, just today this became an issue for one of our internal product teams and I debugged it to the same conclusion. The workaround should be suitable - I wonder if we can apply this automatically for users who do set both the lineSpacing and maxNumberOfLines, until there is a better fix?

[appleguy]

@maicki Shouldn't we set this lineBreakMode to Tail by default, since it is overridden by the attributed string anyway?

I think this sounds right, because it is the default mode for TextKit internally / NSAttributedString, so it would be like our local property matching that default?

/**
 @abstract Determines how the text is truncated to fit within the receiver's maximum size.
 @discussion Defaults to NSLineBreakByWordWrapping.
 @note Setting a truncationMode in attributedString will override the truncation mode set here.
 */
@property (nonatomic, assign) NSLineBreakMode truncationMode;

[maicki]

@appleguy Yep that makes sense and I think I would be in favor of it. Not only TextKit also UILabel's lineBreakMode has Tail as default. That said it would be a breaking API change that could result in some significant behavior change, so I don't know for sure when would be the good time for doing that.

[appleguy]

Interesting consideration. To make the call, I suppose we'd want to think about:

• What cases exist that would be affected by the change?
• In particular, would there be any difference for users who don't have lineSpacing set, for all values of maxNumberOfLines?
• For use cases that would be affected, how likely would it be to affect the engineer's expectations / correctness of their layout?

We should definitely be careful with such a change, and I completely support your decision on this. Here's my thinking:

• Since we believe this only applies when lineSpacing is set, at least a majority of usages should be unaffected.
• Most importantly, of the uses that do set lineSpacing, it's very likely that they are intending this adjusted behavior.
• This is because the bug is fairly subtle / hard to notice, and even harder to debug / do anything about. Users that hit it are likely to waste a lot of time trying to figure it out; the docs help with this, but still there's a risk.

So, one possibility would be to make a change in the next 2.x update with it called out prominently at the top of the release notes. Developers who have a genuine problem caused by it could delay their update while investigating, while we can then be confident that some set of other developers will save time.

Another option: we could add a prominent comment to the header file above lineSpacing or near the Truncation pragma mark, referencing the documentation on this issue.

Your call - if there's anything I can do to help, just let me know!

[ryanfitz]

I'm not certain if this is the same bug, but I am attempting to upgrade an app from ASDK 2.2.1 to texture 2.3.1 and have about 100 snapshot tests failing where it appears the difference is the line heights of ASTextNode's. In my case I am using NSAttributedString's with customized fonts, kerning and line spacing.