Undo/redo - Apply text format to a link breaks the undo functionality

[fluiddot]

Describe the bug
After applying any text format (bold, italic or strikethrough) on a previously created link, the undo action stops working.

To Reproduce
Steps to reproduce the behavior:

1. Add a a rich-text based component with content (Paragraph, Heading, Quote, Media Caption, etc...).
2. Write a word and select it.
3. Press the link format button.
4. Fill the options and dismiss to add a link.
5. Select the word.
6. Press the bold, italic or strikethrough button to apply text format.
7. Press the Undo button.
8. Observe that the text format remains.

Expected behavior
Undo action should revert the text format.

Screenshots

undo-text-forma-link-android.mp4

Smartphone (please complete the following information):

• Device: Samsung Galaxy S20 5G
• OS: Android 10
• Version 16.7-rc-1

Additional context
N/A

[hypest]

👋 Carlos!

Judging from the video, it looks like we can add a No8 step with: "8. Nothing happens"?, right? Kinda a nit but, it's better to have that last step "spelled out" to describe what's the behavior the tester should see when following the reproduction steps, thanks!

[hypest]

Added the "[Prio] High" label since this breaks a core flow (undo/redo). @fluiddot please plan to have a deeper look into this, thanks!

[dcalhoun]

I debugged this some during Groundskeeping. I did not make it to a fix, but wanted to capture what I observed so far.

It would appear that two EDIT_ENTITY_RECORD actions are triggered when bolding a link, as opposed to one action when bolding a word that is not a link. From inspecting the meta data from the two actions, I noticed that the placement of the <strong> tag differs between to the events. When the Bold button is pressed the <strong> tag is inserted within the <a> tag. The second action relocates the <strong> tag to wrap the outside of the <a> tag.

First Action

{
  "type": "EDIT_ENTITY_RECORD",
  "kind": "postType",
  "name": "post",
  "recordId": 1,
  "edits": {
    "blocks": [
      {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "name": "core/paragraph",
        "isValid": true,
        "validationIssues": [],
        "originalContent": "<p>Hello <a href=\"http://www.wordpress.org\" target=\"_blank\" rel=\"noreferrer noopener\">World</a></p>",
        "attributes": {
          "content": "Hello <a rel=\"noreferrer noopener\" href=\"http://www.wordpress.org\" target=\"_blank\"><strong>World</strong></a>",
          "dropCap": false
        },
        "innerBlocks": []
      }
    ],
    "selection": {
      "selectionStart": {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "attributeKey": "content",
        "offset": 6
      },
      "selectionEnd": {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "attributeKey": "content",
        "offset": 11
      },
      "initialPosition": null
    },
    "content": [Function]
  },
  "transientEdits": { "blocks": true, "selection": true },
  "meta": {
    "undo": {
      "kind": "postType",
      "name": "post",
      "recordId": 1,
      "edits": {
        "blocks": [
          {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "name": "core/paragraph",
            "isValid": true,
            "validationIssues": [],
            "originalContent": "<p>Hello <a href=\"http://www.wordpress.org\" target=\"_blank\" rel=\"noreferrer noopener\">World</a></p>",
            "attributes": {
              "content": "Hello <a rel=\"noreferrer noopener\" href=\"http://www.wordpress.org\" target=\"_blank\">World</a>",
              "dropCap": false
            },
            "innerBlocks": []
          }
        ],
        "selection": {
          "selectionStart": {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "attributeKey": "content",
            "offset": 0
          },
          "selectionEnd": {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "attributeKey": "content",
            "offset": 0
          },
          "initialPosition": null
        },
        "content": [Function]
      },
      "transientEdits": { "blocks": true, "selection": true }
    }
  }
}

Second Action

{
  "type": "EDIT_ENTITY_RECORD",
  "kind": "postType",
  "name": "post",
  "recordId": 1,
  "edits": {
    "blocks": [
      {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "name": "core/paragraph",
        "isValid": true,
        "validationIssues": [],
        "originalContent": "<p>Hello <a href=\"http://www.wordpress.org\" target=\"_blank\" rel=\"noreferrer noopener\">World</a></p>",
        "attributes": {
          "content": "Hello <strong><a rel=\"noreferrer noopener\" href=\"http://www.wordpress.org\" target=\"_blank\">World</a></strong>",
          "dropCap": false
        },
        "innerBlocks": []
      }
    ],
    "selection": {
      "selectionStart": {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "attributeKey": "content",
        "offset": 6
      },
      "selectionEnd": {
        "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
        "attributeKey": "content",
        "offset": 11
      },
      "initialPosition": null
    },
    "content": [Function]
  },
  "transientEdits": { "blocks": true, "selection": true },
  "meta": {
    "undo": {
      "kind": "postType",
      "name": "post",
      "recordId": 1,
      "edits": {
        "blocks": [
          {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "name": "core/paragraph",
            "isValid": true,
            "validationIssues": [],
            "originalContent": "<p>Hello <a href=\"http://www.wordpress.org\" target=\"_blank\" rel=\"noreferrer noopener\">World</a></p>",
            "attributes": {
              "content": "Hello <a rel=\"noreferrer noopener\" href=\"http://www.wordpress.org\" target=\"_blank\"><strong>World</strong></a>",
              "dropCap": false
            },
            "innerBlocks": []
          }
        ],
        "selection": {
          "selectionStart": {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "attributeKey": "content",
            "offset": 6
          },
          "selectionEnd": {
            "clientId": "d16c2b82-6aa6-4f3a-89fb-b1124b6f9de2",
            "attributeKey": "content",
            "offset": 11
          },
          "initialPosition": null
        },
        "content": [Function]
      },
      "transientEdits": { "blocks": true, "selection": true }
    }
  }
}

This duplicative second action creates the loop that one cannot exit with the Undo button. Stepping backwards with the Undo button causes the second "reformatting" action to trigger again.

At this time, my belief is that this issue may be related to Aztec-Android. I believe the quote below from wordpress-mobile/WordPress-Android#13169 (comment) is likely true regarding this issue as well.

I think this is because of Aztec-Android. There are a number of changes like this that it makes to content (i.e., if you pass an htmls string into the fromHtml and then immediately call toHtml, you will not always get back the exact same string because Aztec tries to clean up the input.

There has been some talk of trying to create a simplified version of Aztec for use with Gutenberg, and I think that's our best chance to address these issues. I think that without a separate version of Aztec for Gutenberg, it will be very difficult to fix these issues without causing regressions in the classic editor.

I have not proven this theory yet, so it is a possibility that something else is at hand, but given the slight modifications to the block HTML from the rich text field it seems likely.

[fluiddot]

Thanks @dcalhoun for capturing all these useful findings ❤️ !

I've debugged the issue too but focused on the native side of Aztec. I confirm that this is coming from Aztec-Android and is actually related to the wordpress-mobile/WordPress-Android#13169 (comment) comment.

Aztec-Android can perform modifications in the HTML provided by the RichText component which as previously mentioned, results in two actions.

Example:

• Original HTML:
  <p>This is a <a href="http://wordpress.com">link</a></p>
• Undo action 1 - Setting link bold:
  <p>This is a <a href="http://wordpress.com"><strong>link</strong></a></p>
• Undo action 2 - HTML modified by Aztec:
  <p>This is a <strong><a href="http://wordpress.com">link</a></strong></p>

Since this is done at a native level, when the undo action is triggered (Undo action 2), although the HTML is modified, Aztec modifies it again, generating another action like Undo action 2. This behavior is repeated every time when trying to undo, so it prevents the user from executing further undo actions as Aztec will always generate a new one.

In the following items I added refereces to the flow related to how the RichText content is updated and HTML is modified in Aztec:

• Setting HTML in setTextfromJS when the text props changes (reference)
• HTML is parsed and potentially modified in fromHtml (reference)
• HTML is modified to match Aztec style (reference)

---

I lack context about whether we require Aztec to modify the HTML, maybe this is behavior inherited from the past when it was used as the editor, but now that we have the RichText component I think all potential modifications should be handled by that component and not Aztec.

Being this said, as a workaround for this issue, we could skip applying the Aztec style, done in the switchToAztecStyle function, when setting the HTML from Gutenberg to prevent unexpected changes to the HTML code. Although first, we should investigate further if the code executed in switchToAztecStyle (reference) is actually performing any required transformation.

[fluiddot]

I've just opened the following PRs that addresses the issue:

• Add flag to control whether applying Aztec styles to HTML AztecEditor-Android#944
• [RNMobile][Android] Fix undo/redo functionality in links when applying text format WordPress/gutenberg#36766

Additionally, I gathered further investigations in WordPress/gutenberg#36766 regarding the modifications that Aztec-Android does related to the links.

@hypest @dcalhoun I'd appreciate it if you could take a look, thank you very much for the help 🙇 .