[ASTextKitRenderer] Potential fix headIndent attribute not being accounted for by the shadower.

[aaronschubert0]

Hey @appleguy This is to fix the bug specified in #1076. I'm not entirely sure this is the correct place in the stack to fix this issue. The issue stems from the fact that the headIndent property of NSAttributedString actually alters the origin of the boundRect rather than just increase it's size. This bug fix address this by incorporating the origin offset into the final size calculation.

The tailIndent property seems to break things even more, that can be fix in a future diff (haven't got to the bottom of that)

[appleguy]

@aaronschubert0 this is fascinating. Thank you for researching this. It is an extremely important issue, yet there are very few folks prepared to debug the text system.

What kind of test case(s) do you have that helped you diagnose the issue? My top priority is getting the fix in, especially since it looks like I'm going to do a 1.9.5.1 shortly (unfortunately - see #1083). However, if you have appropriate test cases, I want to at least file an issue to ensure that we add a snapshot test to verify this behavior in the face of future text changes.

Could you explain why the new calculation is correct? Why exactly would we add the origin value to the size, in order to generate a size?

[appleguy]

Going to take this now and start testing it in Pinterest, but would still like to continue discussing it - feel free to send me an email if you leave a comment here (it would be good to have the discussion here though so that it is documented).

[aaronschubert0]

Hey @appleguy awesome. Yeah it's definitely a crucial area to invest time in. The test case that I used was the example project linked in #1076 and the debugger. So I definitely need to write some actual tests. Basically it seems that most properties of NSAttributedString are interpreted properly by our text size calculations since they change the size of the text container to achieve their 'effect'. In contrary headIndent (and I assume tailIndent) achieves it's effect by altering the origin of the whole text container, rather than altering the size, so .headIndent = 10 will result in the size to remain the same but the origin will now be (10, 0) rather than (0, 0). Since the positioning of our nodes are usually done by the parent (i.e in a layoutSpec method) it is appropriate to just alter the resulting size of textNode since that will result in the desired affect (that was my thinking behind the change anyway)