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

Unexpected content changes toggling Classic editor from Text to Visual to Text #4672

Closed
2 tasks
bobbingwide opened this issue Jan 25, 2018 · 15 comments
Closed
2 tasks
Assignees
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention
Milestone

Comments

@bobbingwide
Copy link
Contributor

bobbingwide commented Jan 25, 2018

Issue Overview

When using the Classic Editor editing a post detected as Gutenberg, strange things happen to the content when toggling between Text and Visual.

I've seen unwanted paragraphs being created and content disappearing.

This is a backward compatibility problem.

Steps to Reproduce (for bugs)

  1. Edit a new empty post with the Classic editor, Text view.
  2. From the post list open in the Classic editor
  3. Paste in the following
<!-- wp:core/shortcode -->
[bw_table fields=title,featured]
<!-- /wp:core/shortcode -->
  1. Toggle to Visual and back again. Looks OK.
  2. Update
  3. Now toggle again. Unwanted paragraph tags appear around the shortcode.

Expected Behavior

When using the Classic editor you should be able to toggle between Visual and Text without changes to the content.

This unwanted behaviour happens even when the post type being edited does not have 'show_in_rest'.

Current Behavior

Possible Solution

Screenshots / Video

image
image

Related Issues and/or PRs

Todos

  • Tests
  • Documentation
@bobbingwide bobbingwide changed the title Content disappears toggling Classic editor from Text to Visual Unexpected content changes toggling Classic editor from Text to Visual to Text Jan 25, 2018
@bobbingwide
Copy link
Contributor Author

Scenario 2

Blank appearing in empty divs.

When any HTML comment is added to the content that Gutenberg treats as being a block then toggling between Text and Visual adds an unwanted blank in empty divs.
e.g.

<div class="my-content"></div>
<!-- wp:jsforwp/register-demo -->
<div class="my-content"></div>
<!-- /wp:jsforwp/register-demo -->

becomes

<div class="my-content"> </div>
<!-- wp:jsforwp/register-demo -->
<div class="my-content"> </div>
<!-- /wp:jsforwp/register-demo -->
  • this is a backward compatibility issue in the Classic editor.
  • It affects post types which are not 'show_in_rest'.

@bobbingwide
Copy link
Contributor Author

bobbingwide commented Jan 25, 2018

Scenario 3

Classic editor fails to complete loading.
Here's a block taken from Zac Gordon's course. https://github.com/zgordon/gutenberg-course

<!-- wp:jsforwp/register-demo -->
<div class="wp-block-jsforwp-register-demo">
<h2>Block Demo</h2>
<div class="my-content"></div>
</div>
<!-- /wp:jsforwp/register-demo -->
  1. Deactivate Gutenberg
  2. Create a new post and paste the above content in Text edit.
  3. Save
  4. Switching from Text to Visual then back again loses the first comment. See note 1.
  5. Activate Gutenberg and the Gutenberg-course plugin that registers the block
  6. Edit the post using the Classic Editor
  7. Attempt to switch to Visual.
  8. The Visual editor does not load.

Notes: I think that the unexpected removal of the first comment line is:

  1. A core TinyMCE bug ( noted in 4.9.2 and 5.0-alpha-42125-src)
  2. But it's not a problem when Gutenberg is active.

My server log shows a lot of file requests being passed to WordPress
The problem goes away if I comment out the code that enqueues the front-end JavaScript file, called in response to enqueue_block_assets.

    wp_enqueue_script(
        'oik_block-blocks-frontend-js',
        plugins_url( $blockPath, __FILE__ ),
        [ 'wp-i18n', 'wp-element', 'wp-blocks', 'wp-components', 'wp-api' ],
        filemtime( plugin_dir_path(__FILE__) . $blockPath )
    );

I don't understand why gutenberg_common_scripts_and_styles() is invoking this action when it's the classic-editor. REQUEST_URI=/hm/wp-admin/post.php?post=20905&action=edit&classic-editor

@bobbingwide
Copy link
Contributor Author

bobbingwide commented Jan 25, 2018

For Scenario 1 and 2 the workaround/fix seems to be to do the opposite of the logic in
gutenberg_disable_editor_settings_wpautop
by setting wpautop to true.
or to de-register the filter function added in compat.php

remove_filter( 'wp_editor_settings', 'gutenberg_disable_editor_settings_wpautop' );

This change has a different side effect. Whereas p tags are not created around the shortcodes
those which do exist in are converted to blank lines.

<!-- wp:core/paragraph -->
<p>Now I am writing a paragraph and all is fine. In a moment I&#x27;ll switch back to the Classic editor and see what&#x27;s what.</p>
<!-- /wp:core/paragraph -->

After the round trip from Text to Visual and back the content becomes.

<!-- wp:core/paragraph -->

Now I am writing a paragraph and all is fine. In a moment I'll switch back to the Classic editor and see what's what.

<!-- /wp:core/paragraph -->

@danielbachhuber
Copy link
Member

@bobbingwide Is this issue still valid?

@danielbachhuber
Copy link
Member

I can still reproduce the original issue as of master (to be Gutenberg 3.1):

paragraphshortcodes

@danielbachhuber
Copy link
Member

@tofumatt This could use an e2e test if you want to write one for it.

@tofumatt
Copy link
Member

tofumatt commented Jul 9, 2018

From the issue history here it seems this is still relevant, so I can write a failing e2e test and mark it to be skipped, but the actual issue isn't resolved, correct?

@danielbachhuber
Copy link
Member

From the issue history here it seems this is still relevant, so I can write a failing e2e test and mark it to be skipped, but the actual issue isn't resolved, correct?

Correct, the actual issue isn't yet resolved. If you're in the mode of writing e2e tests though, I think this would be high-value to have covered.

@danielbachhuber
Copy link
Member

This issue has been flagged as high-value by Bluehost for Try Gutenberg rollout.

@danielbachhuber danielbachhuber added the [Priority] High Used to indicate top priority items that need quick attention label Jul 11, 2018
@aduth
Copy link
Member

aduth commented Jul 12, 2018

This was meant to be at least partially addressed with the following snippet introduced with #2708:

gutenberg/lib/compat.php

Lines 72 to 87 in 322a3fc

/**
* 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' );

In brief testing, we are passing the setting, though apparently it's not enough to stop the forced paragraphs from being introduced.

I wonder if these tags may be introduced via TinyMCE's forced_root_block setting. It's not clear to me how we could exempt blocks from this without otherwise diminishing the behavior of the editor.

@danielbachhuber
Copy link
Member

@aduth @azaozz Crazy idea: Disable Visual mode in the Classic Editor if blocks are detected. There would be a notice communicating the behavior and the user would have to remove all block markup in order to restore Visual mode.

@aduth
Copy link
Member

aduth commented Jul 18, 2018

Not that crazy. It would certainly lose some convenience, but I imagine it's better to lose the convenience than the data loss (and subsequent Gutenberg-blaming) associated with TinyMCE's destructiveness toward the blocks structure.

@aduth aduth removed this from the 3.3 milestone Jul 18, 2018
@aduth aduth added this to the 3.4 milestone Jul 18, 2018
@mtias mtias modified the milestones: 3.4, Try Callout Jul 19, 2018
@danielbachhuber
Copy link
Member

@aduth @azaozz @pento I put together a patch in https://core.trac.wordpress.org/ticket/44617 that modifies TinyMCE 'BeforeSetContent' and 'SaveContent' behavior to strip <p> paragraph tags from Shortcode Blocks. In this scenario, #2708 would be reverted.

@danielbachhuber
Copy link
Member

@WordPress/gutenberg-core Per the Slack conversation (on Saturday, sorry for working on the weekend), what's the plan moving forward with regards to opening Gutenberg posts in the Classic Editor?

@danielbachhuber
Copy link
Member

danielbachhuber commented Jul 26, 2018

@pento, @aduth and I chatted about this on Zoom today. Rather than try to get TinyMCE to work with Gutenberg posts, we're going to create a warning UX instead #8213

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Development

No branches or pull requests

7 participants