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

Classic Editor fails to load in Visual edit #4678

Closed
2 tasks
bobbingwide opened this issue Jan 25, 2018 · 9 comments
Closed
2 tasks

Classic Editor fails to load in Visual edit #4678

bobbingwide opened this issue Jan 25, 2018 · 9 comments
Labels
Needs Testing Needs further testing to be confirmed. [Status] Needs More Info Follow-up required in order to be actionable.

Comments

@bobbingwide
Copy link
Contributor

bobbingwide commented Jan 25, 2018

Issue Overview

If an extension plugin responds to enqueue_block_assets and it enqueues its own script, with dependencies on Gutenberg's scripts, then this can break the Classic Editor in Visual mode.

Steps to Reproduce (for bugs)

  1. Deactivate Gutenberg
  2. Create a new post in Text mode with any content you like.
  3. Save
  4. Activate Gutenberg and a plugin that responds to enqueue_block_assets using code similar to below.
  5. Edit the post using the Classic Editor.
  6. Attempt to switch to Visual.
  7. The Visual editor does not load.
    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 )
    );

Expected Behavior

You should be able to toggle between the Text and Visual editor modes without failure.

Current Behavior

You can merrily edit in Text mode, but attempting to switch to Visual will lead to an empty TinyMCE window with no icons. Once this has happened it's very difficult to get back to Text without deactivating a plugin or two.

Possible Solution

When using the Classic Editor there should be no need for Gutenberg blocks.
The classic-editor parameter is passed in the request and is available from $_GET.

A new test is required to avoid invoking enqueue_block_assets.

As a workaround, for my plugin, I added the test to my own action hook function.

Screenshots / Video

image

Related Issues and/or PRs

This was originally raised as Scenario 3 in #4672.

Todos

  • Tests
  • Documentation
@bobbingwide
Copy link
Contributor Author

bobbingwide commented Feb 14, 2018

There's a similar problem with the Text Widget.
We'll have to put some more work into PR 4693.

issue-4678-text-widget-problem

This animated gif (sorry, it's a bit fast) shows

  • the Text widget displaying correctly, when gutenberg_common_scripts_and_styles was not being called.
  • I then re-enabled the function and refreshed the page.
  • The TinyMCE display is not working.
    image
    There are lots of messages in the Console log

image

The easiest way to fix this appears to be to remove

add_action( 'admin_enqueue_scripts', 'gutenberg_common_scripts_and_styles' );

@aduth
Copy link
Member

aduth commented Feb 21, 2018

The easiest way to fix this appears to be to remove

add_action( 'admin_enqueue_scripts', 'gutenberg_common_scripts_and_styles' );

Wouldn't this break all blocks which enqueue for the actual Gutenberg editor screen?

@bobbingwide
Copy link
Contributor Author

bobbingwide commented Feb 27, 2018

Wouldn't this break all blocks which enqueue for the actual Gutenberg editor screen?

No, see the table that I have created in bobbingwide/oik-block#8

These blocks are enqueued in response to enqueue_block_editor_assets, which gets called by gutenberg_editor_scripts_and_styles() which is attached to admin_enqueue_scripts if replace_editor is true.

@bobbingwide
Copy link
Contributor Author

I've revisited this issue with an update to the Text Widget part of the problem.
First, I changed my plugin to register the scripts in response to init and only enqueue them when safe.

When responding to enqueue_block_editor_assets it checks what's happening.

if ( doing_filter( "replace_editor" ) ) {
  wp_enqueue_script( 'oik_block-blocks-js' );
}

When responding to enqueue_block_assets it checks that it's the frontend.

if ( is_admin() ) {
  return;
}
wp_enqueue_script( ...

But when the scripts are defined using register_block_type

register_block_type( 'oik-block/dummy', 
  [ 'render_callback' => 'oik_block_dummy' ,	
  'editor_script' => 'oik_block-dummy-js', 
  'script' => 'oik_block-dummy-js'
  ] );

and the script has been registered with dependencies on [ 'wp-blocks', 'wp-element' ] then
the problem with the Text Widget still occurs.

The fix for this is either to not register scripts that have these dependencies
or to change gutenberg_enqueue_registered_block_scripts_and_styles
to only enqueue them in the front-end.

if ( ! empty( $block_type->script ) && !is_admin() ) {
  wp_enqueue_script( $block_type->script );
}

Comments appreciated.

@aduth
Copy link
Member

aduth commented May 7, 2018

@bobbingwide Could you boil this down to a minimal reproducible case? I'm trying to reproduce unsuccessfully, registering a block during init which includes [ 'wp-blocks', 'wp-element' ] in its script dependencies, but am not observing the same issues.

@bobbingwide
Copy link
Contributor Author

bobbingwide commented May 7, 2018

@aduth Just to confirm that the Classic editor loads but the Text widget doesn't. Have you tried that part of the problem?

@aduth
Copy link
Member

aduth commented May 7, 2018

Yes, I tried using the Text Widget and it loaded okay in my minimal example. Here's the code I have in its entirety:

<?php
/*
 * Plugin Name: Demo Block
 */

function demo_block_init() {
	wp_register_script(
		'demo-block',
		plugins_url( 'demo-block.js', __FILE__ ),
		[ 'wp-blocks', 'wp-element' ]
	);

	register_block_type( 'demo/demo-block', array(
		'editor_script' => 'demo-block',
	) );
}
add_action( 'init', 'demo_block_init' );

(Tried both script and editor_script)

@bobbingwide
Copy link
Contributor Author

Your code fails for me when it's register_block_type with 'script' => 'demo-block'. Still getting the same console messages as in #4678 (comment)

@danielbachhuber
Copy link
Member

@bobbingwide Is this still an issue?

@danielbachhuber danielbachhuber added the [Status] Needs More Info Follow-up required in order to be actionable. label Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Testing Needs further testing to be confirmed. [Status] Needs More Info Follow-up required in order to be actionable.
Projects
None yet
Development

No branches or pull requests

4 participants