Rich Text: Remove restoreContentAndSplit

[aduth]

Related: https://github.com/WordPress/gutenberg/pull/5100/files#r198953121

This pull request seeks to simplify the RichText component, removing its restoreContentAndSplit function which had been used to preserve content through a block split. In my testing, I had found that this was not a behavior caused by TinyMCE, as indicated by the function's comment, but rather in our use of Range#extractContents which moves (i.e. destroys) contents of a range. Updating this code to use Range#cloneContents instead avoids the need to re-set the RichText content which, in turn, should be a boon to performance and avoid the need for this pass-through function.

Testing instructions:

Verify that there are no regressions in block splitting. Particularly, ensure that a paragraphs contents are not destroyed after pressing Enter within.

[aduth]

Funny enough, this will incidentally fix the issue described at #5095 (comment), which is still technically an issue for updateContent, but since we don't call it anymore on the split, works for the given steps.

Some related discussion today in Slack with @tofumatt

[aduth]

The end-to-end test failure is a legitimate bug where the caret is in the first paragraph wrongly when splitting in two from the middle: But in fact it's the same bug observed at #5095 (comment) and proposed to be fixed by #7620 . You can observe that it becomes fixed again if you cherry-pick 29bcba5 into your local branch.

Thus, considering #7620 as a blocker for this.

[ellatrix]

Could the afterFragment still be extracted, so we can avoid having to set it afterwards? I think this this happens in the block edit component. Or maybe I'm misunderstanding this.

[aduth]

so we can avoid having to set it afterwards?

What do you mean by set?

[ellatrix]

By means of setAttributes in the block edit function.

[aduth]

Still not totally sure I understand, but we should want the explicit setAttributes and avoid leveraging any of these more implicit DOM mutations.

[gziolo]

Thus, considering #7620 as a blocker for this.

It looks like #7620 was resolved, can we rebase this one and merge?

[aduth]

Rebased and force pushed. Running locally, all tests / functionality appears to work as expected now.

[tofumatt]

I tested this locally, split a bunch of blocks, paragraphs, lists, headings, including paragraphs that had bold characters and such–split them inside and outside the bold characters.

All seemed to work well and I encountered no bugs.

Writing a few e2e tests for block splitting would be good, I dunno if we have any. Either way I suppose that's not just related to this PR 😄

🚢

[tofumatt]

[aduth]

Writing a few e2e tests for block splitting would be good, I dunno if we have any. Either way I suppose that's not just related to this PR 😄

There's quite a few split across splitting-merging.test.js and writing-flow.test.js, though I'm open to adding more if there's a problematic flow not currently covered.