Text is splitted into two paragraphs when switching editor tabs

[ocean90]

Probably introduced by #41962/#42029.

Steps to reproduce:

• Go to the edit screen of a post
• Set cursor in the middle of a sentence
• Switch to text tab
• Switch back to visual tab

Video: ​https://cloudup.com/cVoCok2lmNx

Expected behaviour: No additional paragraph is added.

[ocean90]

@biskobe Is this something you can look into?

[adamsilverstein]

Introduced in r41783 according to git bisect.

[afercia]

Given also #42530 was introduced by the "keep selection" changes, and today it's the day before the 4.9 release, I'd be +1 for reverting the changes. They are just not tested enough and this is a lesson we should learn. FWIW, I wouldn't be so comfortable with shipping WordPress 4.9 with these 2 very evident regressions in a so critical area like the editor.

[adamsilverstein]

42531.diff​ reverts r41783 and fixes the issue in my testing.

Given the size/scope of this change, and its recently discovered unintended consequences, and the timing of the 4.9 release, reverting probably makes the most sense here.

[aduth]

The issue appears to occur during the switch from Text to Visual mode, when parsing the textarea value, specifically here:

​https://github.com/tinymce/tinymce/blob/7d7a8db05d55bd5078849c23a342993588ce9995/src/core/src/main/js/html/DomParser.js#L351-L352

With the changes introduced in r41783, a span with attribute data-mce-type="bookmark" is inserted into the textarea value when switching from Text to Visual modes, with intention of preserving the caret position.

​https://github.com/WordPress/wordpress-develop/blob/f7dabb40c1293cb6490d4fc16e6c2b14cfbbc0e8/src/wp-admin/js/editor.js#L528

When parsing, TinyMCE considers three nodes: the text node before and after the span bookmark, and the span bookmark itself. With the logic of the DOMParser, it ends the first root block when encountering the span assigned a data-mce-type, thereby resulting in the creation of two root nodes (the two paragraphs from the report).

In brief testing, omitting the behavior to add the data-mce-type attribute when creating the cursor marker resolves the issue of two paragraphs being created.

diff --git src/wp-admin/js/editor.js src/wp-admin/js/editor.js
index ed77d3c755..140b258b6f 100644
--- src/wp-admin/js/editor.js
+++ src/wp-admin/js/editor.js
@@ -525,7 +525,7 @@ window.wp = window.wp || {};
                                mode = htmlModeCursorStartPosition !== htmlModeCursorEndPosition ? 'range' : 'single',
 
                                selectedText = null,
-                               cursorMarkerSkeleton = getCursorMarkerSpan( $$, '' ).attr( 'data-mce-type','bookmark' );
+                               cursorMarkerSkeleton = getCursorMarkerSpan( $$, '' );
 
                        if ( mode === 'range' ) {
                                var markedText = textArea.value.slice( htmlModeCursorStartPosition, htmlModeCursorEndPosition ),

[afercia]

Playing with text, switching multiple times, re-unifying the two split lines in just one line, then switching again visual/text etc. I also randomly get a span in the content. Using Chrome and Safari:
This i<span style="font-size: 1rem;">s a sentence</span>

[peterwilsoncc]

I'm +1 on reverting with 42531.diff​ too.

[pento]

42531.2.diff​ fixes this bug. autop() needed to wrap single line posts in <p> tags, so that, as @aduth noted, TinyMCE doesn't try to wrap the text nodes in <p> tags as it parses the post DOM.

[peterwilsoncc]

42531.2.diff​ is doing what it says on the pack.

No paragraph is added switching between tabs.

[dd32]

It looks like the code in 42531.2.diff​ is really only an optimisation, to skip processing text when not needed (as TinyMCE will wrap it in a paragraph tag anyway).
Removing that shouldn't cause any issues from what I can see, +1 from me.

[dd32]

Yay for trac showing warnings off screen :) (removing tag)

[peterwilsoncc]

In 42177:

Editor: Cease adding paragraphs when switching editor tabs.

Fixes an error since [41783] in which extra paragraphs could be added to content when switching editor tabs.

Props pento, adamsilverstein, aduth.
Fixes #42531.

[westonruter]

In 42178:

Editor: Cease adding paragraphs when switching editor tabs.

Fixes an error since [41783] in which extra paragraphs could be added to content when switching editor tabs.

Props pento, adamsilverstein, aduth.
Fixes #42531 for 4.9.

[aduth]

Replying to dd32:

It looks like the code in 42531.2.diff​ is really only an optimisation, to skip processing text when not needed (as TinyMCE will wrap it in a paragraph tag anyway).
Removing that shouldn't cause any issues from what I can see, +1 from me.

Noting that there is a change in behavior after 42531.2.diff

Before:

switchEditors.wpautop( 'Foo' );
// "Foo"

After:

switchEditors.wpautop( 'Foo' );
// "<p>Foo</p>\n"

However, the "After" case now more closely mimics the PHP behavior (which has the same result). Still worth noting that there is a difference with potential impact.

[aduth]

Another effect of changes introduced in r42178:

Before:

switchEditors.wpautop( '' );
// ""

After:

switchEditors.wpautop( '' );
// "<br />\n"

In this instance, I'd consider it a regression, since the "Before" behavior matches that found in PHP's wpautop.