Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.9.1: Classic "Visual" editor litters Gutenberg markup w/ superfluous `<p>´ tags #3901

Closed
lkraav opened this issue Dec 10, 2017 · 11 comments
Closed
Assignees
Labels
[Type] Bug An existing feature does not function as intended

Comments

@lkraav
Copy link

lkraav commented Dec 10, 2017

(EDIT still broken in 2.3.0)
(EDIT 2018-07-04 improved in 3.1.1, shortcodes still broken)
(EDIT 2018-07-21 ^^^^ 3.3.0)

Issue Overview

Classic "Visual" editor litters Gutenberg markup w/ superfluous <p> tags.
This is an issue where occasionally the user might have to revert to using the classic editor, due to Gutenberg metabox rendering JS crashing for whatever reason. Even if the user doesn't touch the content block at all, just loading Gutenberg markup in classic Visual editor will cause it to break on Update.

I do not have Classic Editor plugin activated. Unless this is officially going to be a "run Gutenberg" requirement, I probably shouldn't have to activate it, right?

Steps to Reproduce (for bugs)

  1. Author a new Gutenberg post with this example markup
<!-- wp:heading -->
<h2>Shinier than yours, meatbag.</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>You can crush me but you can&#x27;t crush my spirit! Is today&#x27;s hectic lifestyle making you tense and impatient? It&#x27;s a T. It goes &quot;tuh&quot;. I&#x27;ll tell them you went down prying the wedding ring off his cold, dead finger.</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[column grid="2" span="1"]
<!-- /wp:shortcode -->

<!-- wp:paragraph -->
<p>I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don&#x27;t like lilacs! Your &#x27;first&#x27; wife was the one who liked lilacs! No argument here. A true inspiration for the children.</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[/column]
<!-- /wp:shortcode -->

<!-- wp:shortcode -->
[column grid="2" span="1"]
<!-- /wp:shortcode -->

<!-- wp:paragraph -->
<p>Hello world.</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[/column]
<!-- /wp:shortcode -->
  1. Open this post in classic editor, switch to Visual mode
  2. Switch back to Text mode to review littered markup

Expected Behavior

Gutenberg markup stays intact

Current Behavior

Littered markup result - shortcodes are wrapped into <p> tags, all block marker comments empty lines get replaced w/ <p>&nbsp;</p>:

<!-- wp:core/heading -->
<h2>Shinier than yours, meatbag.</h2>
<!-- /wp:core/heading -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>You can crush me but you can't crush my spirit! Is today's hectic lifestyle making you tense and impatient? It's a T. It goes "tuh". I'll tell them you went down prying the wedding ring off his cold, dead finger.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>Hello world.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->

Possible Solution

Not sure which component is the responsible party - core or Gutenberg - could use pointers.

Screenshots / Video

Related Issues and/or PRs

Maybe #3900? Not sure, could use pointers.

@lkraav
Copy link
Author

lkraav commented Dec 11, 2017

@norewp next step here is to test my two bug scenarios w/ the freshly released 1.9.0.

@lkraav lkraav changed the title 1.8.1: Classic "Visual" editor litters Gutenberg markup w/ superfluous `<p>´ tags 1.9.0: Classic "Visual" editor litters Gutenberg markup w/ superfluous `<p>´ tags Dec 11, 2017
@lkraav
Copy link
Author

lkraav commented Dec 11, 2017

Tested on 1.9.0, situation is exactly the same.

@lkraav lkraav changed the title 1.9.0: Classic "Visual" editor litters Gutenberg markup w/ superfluous `<p>´ tags 1.9.1: Classic "Visual" editor litters Gutenberg markup w/ superfluous `<p>´ tags Dec 14, 2017
@aduth
Copy link
Member

aduth commented Dec 18, 2017

Making a note to test this behavior in #4005 if we move in the direction of client-side wpautop . Seems this is where things are likely breaking down. Alternatively the selective wpautop which is currently implemented server-side ought to be trimming whitespace between blocks:

gutenberg/lib/blocks.php

Lines 193 to 232 in 6a287a4

/**
* Given a string, returns content normalized with automatic paragraphs applied
* to text not identified as a block. Since this executes the block parser, it
* should not be used in a performance-critical flow such as content display.
* Block content will not have automatic paragraphs applied.
*
* @since 1.7.0
*
* @param string $content Original content.
* @return string Content formatted with automatic paragraphs applied
* to unknown blocks.
*/
function gutenberg_wpautop_block_content( $content ) {
$blocks = gutenberg_parse_blocks( $content );
foreach ( $blocks as $i => $block ) {
if ( isset( $block['blockName'] ) ) {
continue;
}
$content = $block['innerHTML'];
// wpautop will trim leading whitespace and return whitespace-only text
// as an empty string. Preserve to apply leading whitespace as prefix.
preg_match( '/^(\s+)/', $content, $prefix_match );
$prefix = empty( $prefix_match ) ? '' : $prefix_match[0];
$content = $prefix . wpautop( $content, false );
// To normalize as text where wpautop would not be applied, restore
// double newline to wpautop'd text if not at the end of content.
$is_last_block = ( count( $blocks ) === $i + 1 );
if ( ! $is_last_block ) {
$content = str_replace( "</p>\n", "</p>\n\n", $content );
}
$blocks[ $i ]['innerHTML'] = $content;
}
return gutenberg_serialize_blocks( $blocks );
}

@azaozz
Copy link
Contributor

azaozz commented Jan 2, 2018

Perhaps a better alternative would be to turn wpautop off when editing Gutenberg posts in the classic editor. Leaving the <p> tags intact works quite nicely there.

@aduth
Copy link
Member

aduth commented Jan 3, 2018

Perhaps a better alternative would be to turn wpautop off when editing Gutenberg posts in the classic editor. Leaving the <p> tags intact works quite nicely there.

This exists already:

gutenberg/lib/compat.php

Lines 126 to 141 in 91dd36c

/**
* Disables wpautop behavior in classic editor when post contains blocks, to
* prevent removep from invalidating paragraph blocks.
*
* @param array $settings Original editor settings.
* @return array Filtered settings.
*/
function gutenberg_disable_editor_settings_wpautop( $settings ) {
$post = get_post();
if ( is_object( $post ) && gutenberg_post_has_blocks( $post ) ) {
$settings['wpautop'] = false;
}
return $settings;
}
add_filter( 'wp_editor_settings', 'gutenberg_disable_editor_settings_wpautop' );

See: #2708

But we still conversely need to turn wpautop on selectively for legacy posts when loading the Gutenberg editor.

@aduth
Copy link
Member

aduth commented Jan 8, 2018

The root cause here appears to be in TinyMCE's content serializer during content initialization, which converts the comment demarcation sequence to a new paragraph node:

Serialize

(Code reference)

Off the top of my head, a couple ideas:

  • If we could force content to be set as non-raw for the initialization, the serializer would not be applied. We could apply this exclusively to block'd content.
  • We could try to find a format which doesn't trigger TinyMCE's default handling of "comment node + newline + comment node = new forced root paragraph"
  • Internal changes to TinyMCE's serializer to not apply the forced root node from the serializer on these text patterns

cc @spocke

@azaozz
Copy link
Contributor

azaozz commented Jan 8, 2018

Confirmed, also made a fiddle: http://fiddle.tinymce.com/0hgaab/9

Imho both comments and following line breaks should be ignored when forcing root blocks in TinyMCE. There shouldn't be difference between:

<!-- comment -->
<p>a</p>
<!-- comment -->

<!-- comment -->
<p>b</p>
<!-- comment -->

<!-- comment -->
<p>c</p>
<!-- comment -->

and

<!-- comment -->
<p>a</p>
<!-- comment --><!-- comment -->
<p>b</p>
<!-- comment --><!-- comment -->
<p>c</p>
<!-- comment -->

@jeffpaul jeffpaul added the [Type] Enhancement A suggestion for improvement. label Jan 26, 2018
@lkraav
Copy link
Author

lkraav commented Feb 23, 2018

Tested broken on 2.2.0.

EDIT Tested broken 2.3.0.

@aduth aduth added [Type] Bug An existing feature does not function as intended [Component] TinyMCE and removed [Type] Enhancement A suggestion for improvement. labels Feb 28, 2018
@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone May 31, 2018
@danielbachhuber danielbachhuber self-assigned this May 31, 2018
@danielbachhuber
Copy link
Member

Closing in favor of #6385 (comment), which has a proposed solution.

@lkraav
Copy link
Author

lkraav commented Jul 4, 2018

Just to follow up here for at least my own records, 3.1.1 shows improvement.

<p>&nbsp;</p> litter is gone. Unfortunately, shortcodes still become broken (wrapped in <p> tags).

v2.x

<!-- wp:core/heading -->
<h2>Shinier than yours, meatbag.</h2>
<!-- /wp:core/heading -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>You can crush me but you can't crush my spirit! Is today's hectic lifestyle making you tense and impatient? It's a T. It goes "tuh". I'll tell them you went down prying the wedding ring off his cold, dead finger.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->
<p>&nbsp;</p>
<!-- wp:core/paragraph -->
<p>Hello world.</p>
<!-- /wp:core/paragraph -->
<p>&nbsp;</p>
<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->

v3.1.1 (=== 3.3.0, 3.2.0)

<!-- wp:core/heading -->
<h2>Shinier than yours, meatbag.</h2>
<!-- /wp:core/heading -->

<!-- wp:core/paragraph -->
<p>You can crush me but you can't crush my spirit! Is today's hectic lifestyle making you tense and impatient? It's a T. It goes "tuh". I'll tell them you went down prying the wedding ring off his cold, dead finger.</p>
<!-- /wp:core/paragraph -->

<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->

<!-- wp:core/paragraph -->
<p>I daresay that Fry has discovered the smelliest object in the known universe! For the last time, I don't like lilacs! Your 'first' wife was the one who liked lilacs! No argument here. A true inspiration for the children.</p>
<!-- /wp:core/paragraph -->

<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->

<!-- wp:core/shortcode -->
<p>[column grid="2" span="1"]</p>
<!-- /wp:core/shortcode -->

<!-- wp:core/paragraph -->
<p>Hello world.</p>
<!-- /wp:core/paragraph -->

<!-- wp:core/shortcode -->
<p>[/column]</p>
<!-- /wp:core/shortcode -->

@danielbachhuber
Copy link
Member

Unfortunately, shortcodes still become broken (wrapped in <p> tags).

Yes, this is tracked with #4456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants