Retain Heading attribute when headings are autocorrected.

[twstokes]

Related:

• H2 heading block text changed to bold gutenberg-mobile#4063
• [Gutenberg] [Aztec] Prevent headings from becoming bold after words are autocorrected WordPress-iOS#17844

This PR aims to resolve an issue where TextStorage attributed string replacements from the operating system (e.g. autocorrect, keyboard suggestions, double-tapping the spacebar to insert a period) aren't copying all of the existing attributes for the range they're replacing. When this is performed on a Heading, the headingRepresentation attribute key is missing, and ultimately causes the HTML generator to wrap certain strings in <strong> tags. This PR only addresses missing headingRepresentation keys.

I've created a POC to show that may be a bug on Apple's side.

State inspection using the Aztec demo:

When inserting a breakpoint here, outputting the supplied attrString after autocorrect has taken place will show the NSAttributedString doesn't contain the headingRepresentation attribute.

Functionality of these changes:

1. When func replaceCharacters(in range: NSRange, with attrString: NSAttributedString) is called, the provided attributed string is checked to see if it includes a headingRepresentation custom key.
2. If it doesn't, we do another check to see if the first character of the existing storage string textStore includes it. If so, we determine that it's a heading and inject the custom attribute. Otherwise we don't mutate the string.

An edge case may come into question: What if the first word is autocorrected, won't the first character be missing the heading attribute?

This should be no due to the insertion of the initial characters from the keyboard, which carry the heading attribute.

To test:

Add a breakpoint to this line., but don't enable it yet.

1. In the Aztec demo app, load an Empty Demo.
2. Change the input to any Heading.
3. Type: "Hello i'm"
4. Observe that "i'm" is highlighted in blue, indicating that it's about to be autocorrected.
5. Enable the breakpoint.
6. Press the spacebar on the demo app. (the breakpoint should be caught)

7a. Observe the value of attrString. It should equal:

I'm{
    NSColor = "<UIDynamicSystemColor: 0x600000101680; name = labelColor>";
    NSFont = "<UICTFont: 0x7fecd47112e0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x6000014f7a80>]";
}

7b. Observe the value of textStore. It should equal:

Hello i'm{
    NSColor = "<UIDynamicSystemColor: 0x600000a56700; name = labelColor>";
    NSFont = "<UICTFont: 0x7f7fed910640> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x600001fb4840>]";
    headingRepresentation = 1;
}

8. Step to line 304 and observe the value of preprocessedString. It should equal:

I'm{
    NSColor = "<UIDynamicSystemColor: 0x600000101680; name = labelColor>";
    NSFont = "<UICTFont: 0x7fecd47112e0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x6000014f7a80>]";
    headingRepresentation = 1;
}

9. Advance to line 310 and observe the value of textStore. It should equal:

Hello I'm{
    NSColor = "<UIDynamicSystemColor: 0x600000101680; name = labelColor>";
    NSFont = "<UICTFont: 0x7fecd47112e0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x6000014f7a80>]";
    headingRepresentation = 1;
}

Before this change, the output would equal:

Hello {
    NSColor = "<UIDynamicSystemColor: 0x600000a56700; name = labelColor>";
    NSFont = "<UICTFont: 0x7f7fed910640> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x600001fb4840>]";
    headingRepresentation = 1;
}I'm{
    NSColor = "<UIDynamicSystemColor: 0x600000a56700; name = labelColor>";
    NSFont = "<UICTFont: 0x7f7fed910640> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 24.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 8, ParagraphSpacingBefore 8, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    4N,\n    8N,\n    12N,\n    16N,\n    20N,\n    24N,\n    28N,\n    32N,\n    36N,\n    40N,\n    44N\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection -1, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint '(null)', paragraphProperties: [<Aztec.Header: 0x600001fb4840>]";
}

Note that the trailing space doesn't appear until after this text is processed, so it won't be shown in these steps.

[guarani]

I've created a POC to show that may be a bug on Apple's side.

I tested this POC locally and see the same output you shared in this code comment. Looking at that output, the custom attribute seems to be still applied to the "Hello " portion of the string, as well as the a trailing space. Is this how you interpret the output? I may be wrong, but the } { seems to be referring to a trailing space (e.g. "I'm ").

I ask because reading this PR description (which I haven't tested yet), seems to outline a solution that doesn't make reference to what I noted above, so I wanted to ask you if this was relevant.

[twstokes]

I've created a POC to show that may be a bug on Apple's side.

I tested this POC locally and see the same output you shared in this code comment. Looking at that output, the custom attribute seems to be still applied to the "Hello " portion of the string, as well as the a trailing space. Is this how you interpret the output? I may be wrong, but the } { seems to be referring to a trailing space (e.g. "I'm ").

I ask because reading this PR description (which I haven't tested yet), seems to outline a solution that doesn't make reference to what I noted above, so I wanted to ask you if this was relevant.

Thanks for pointing this out @guarani! I've made this clearer by adding better testing steps. Just let me know if anything is unclear or if you notice anything's incorrect. 🙇 The Aztec demo with the new steps should be followed over the POC.

[guarani]

I agree there does appear to be bug in how the UITextView's typingAttributes are cleared. I came across https://stackoverflow.com/questions/38399702/maintain-custom-attributes-when-replacing-attributed-text-on-uitextview, would you agree it matches the issue we're seeing here?

If replaceCharacters is only ever called when text is edited, I would assume that the text's attributes shouldn't change, so we can copy the custom heading attribute across.

If replaceCharacters is called in other scenarios for example when switching from a header to a paragraph, then I suspect the heading attribute might be copied across from the header to the paragraph. To test this, I followed these steps:

1. On the Empty Demo screen, switch to a Header
2. Add a breakpoint to the end of the replaceCharacters function mentioned above
3. Type one or more characters, and (when the breakpoint is hit) notice that textStore contains headingRepresentation = 1; for the all characters (as expected)
4. Convert the Heading to a Paragraph
5. Notice the breakpoint is hit again, print textStore and notice that the headingRepresentation = 1; is still present (not expected)

I'm not sure why visually the text looks like a paragraph even though the heading custom attribute is still there. Also, I tested the Heading block to Paragraph block transformation and it seemed to work, so not sure what the impact is here. Would you say it's a bug? I'm not sure yet.

[guarani]

I also want to share something I noticed while testing this. Take this example (based on your above POC):

import UIKit

class ViewController: UIViewController {
    @IBOutlet weak var textView: UITextView!  // UITextViewDelegate is set via Interface Builder

    override func viewDidLoad() {
        super.viewDidLoad()

        textView.typingAttributes = [
            NSAttributedString.Key.font: UIFont.systemFont(ofSize: 40),
            NSAttributedString.Key.customKey: 1
        ]
    }
}

extension ViewController: UITextViewDelegate {
    func textViewDidChange(_ textView: UITextView) {
        print(textView.textStorage)
    }
}

extension NSAttributedString.Key {
    static let customKey = NSAttributedString.Key("custom")
}

After running the app, I type H into the text view and see the custom = 1; custom attribute logged (as expected).
I then type e and notice that only the H has the custom attribute:

H{
    NSFont = ...
    NSParagraphStyle = ...
    custom = 1;
}e{
    NSFont = ...
    NSParagraphStyle = ...
}

Note that no autocorrect has taken place, and no text has been set programmatically. I'd previously thought that the bug affected only autocorrect (and possibly programatically set text), but this suggests it's present in other scenarios.

[twstokes]

After running the app, I type H into the text view and see the custom = 1; custom attribute logged (as expected).
I then type e and notice that only the H has the custom attribute:

That's a good observation @guarani and the reason why I had to implement setAttributes() throughout the POC. Aztec has a lot of checks throughout to keep the attributes in sync, for example when text is inserted and the current typing attributes are mutated.

In the case of autocorrect this differed slightly in that attempts to force the current typing attributes failed. [textViewShouldBeginEditing, shouldChangeTextIn].

[twstokes]

Also, I tested the Heading block to Paragraph block transformation and it seemed to work, so not sure what the impact is here. Would you say it's a bug? I'm not sure yet.

This is a good catch! We really shouldn't be retaining the headingRepresentation in this scenario, but I don't know what downstream effects (if any) this would cause. It could break assumptions that the parser's making.

Furthermore it seems to "pollute" the myTypingAttributes value. 😐

[twstokes]

@guarani I observed that when headings are used they contain a ParagraphStyle property with the Header type, so I've added a check for this to mitigate the heading attribute carryover when converting to a paragraph.

I've also added a test for this and updated the original test to use a HeaderFormatter.

[guarani]

This is working great! I tested using both the original testing steps and the scenario where a Heading is converted to a Paragraph and both worked as expected.

I also spent some time checking for regressions and was unable to spot any.

Thanks for your awesome work here finding a fix @twstokes! 💯

[twstokes]

Thanks for the review @guarani and for making this work better!