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

Try: Aggressive TinyMCE deprecation #50387

Merged
merged 31 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
12f8647
Add experiment setting field
tyxla May 3, 2023
fc80223
Set a global when experiment enabled
tyxla May 3, 2023
6183e67
Load Classic block only when experiment is not enabled
tyxla May 3, 2023
4ed42c5
Do not enqueue any TinyMCE assets if experiment is enabled
tyxla May 5, 2023
fa36172
Do not enqueue editor if experiment enabled
tyxla May 8, 2023
b2f58f7
Set and utilize a cookie when tinymce is requested
tyxla May 8, 2023
fa0cea3
Safeer cookie access on the server side
tyxla May 8, 2023
ef79a75
Add a script to force TinyMCE usage for testing
tyxla May 8, 2023
f52d567
Move experiment toggle to the higheest priority
tyxla May 8, 2023
6599db7
Add PHP comments and fix coding standards
tyxla May 8, 2023
8084674
Rename class file
tyxla May 8, 2023
1ee79d8
More CS
tyxla May 8, 2023
ddd9dfe
Use gutenberg_is_experiment_enabled()
tyxla May 25, 2023
cf08033
Cleanup lines from rebase
tyxla May 25, 2023
e054060
Backend detection of classic block instances
tyxla May 25, 2023
1799b8a
Comment the cookie testing code again
tyxla May 25, 2023
f06c648
Fix code style
tyxla May 25, 2023
b782ce2
Consider the freeform block, or any raw HTML to be Unknown block
tyxla May 25, 2023
853d0bc
Proper post object existence testing
tyxla May 25, 2023
6c7edf6
Register classic block if server declares it's needed
tyxla May 25, 2023
044cd66
A custom message when converting from classic block
tyxla May 25, 2023
1c9fb52
Improve content length check
tyxla May 25, 2023
72cf3df
Expand comment on classic block registration
tyxla May 25, 2023
57ff691
Move example TinyMCE usage code up
tyxla May 30, 2023
d4d6366
Use a query argument instead of a cookie
tyxla Jun 13, 2023
7fc9a82
Rename proxy trap
tyxla Jun 29, 2023
797093a
Fix the comment
tyxla Jun 29, 2023
ddfec41
Rename another function
tyxla Jun 29, 2023
66ed611
Improve the classic block load conditions comment
tyxla Jun 29, 2023
d448cac
Further polish the conditions comment
tyxla Jun 30, 2023
e5ce4e9
Fix a fatal in the wp-admin dashboard
tyxla Jul 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/client-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,13 @@ function gutenberg_register_packages_scripts( $scripts ) {
// Add dependencies that cannot be detected and generated by build tools.
switch ( $handle ) {
case 'wp-block-library':
array_push( $dependencies, 'editor' );
if (
! gutenberg_is_experiment_enabled( 'gutenberg-no-tinymce' ) ||
! empty( $_GET['requiresTinymce'] ) ||
gutenberg_post_being_edited_requires_classic_block()
) {
array_push( $dependencies, 'editor' );
}
break;

case 'wp-edit-post':
Expand Down
14 changes: 14 additions & 0 deletions lib/experimental/assets/tinymce-proxy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
async function reloadWithTinymce() {
const currentUrl = new URL( window.location.href );
currentUrl.searchParams.set( 'requiresTinymce', '1' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using a query argument in order to be able to detect we need TinyMCE from the server.

window.location.href = currentUrl;
}
tyxla marked this conversation as resolved.
Show resolved Hide resolved

window.tinymce = new Proxy(
{},
{
get: reloadWithTinymce,
set: reloadWithTinymce,
apply: reloadWithTinymce,
}
);
22 changes: 22 additions & 0 deletions lib/experimental/class--wp-editors.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
/**
* Contains the placeholder class to replace the default `_WP_Editors`.
*
* @package gutenberg
* @since 6.3.0
*/

// phpcs:disable PEAR.NamingConventions.ValidClassName.StartWithCapital

/**
* Placeholder class.
* Used to disable loading of TinyMCE assets.
*
* @access public
*/
final class _WP_Editors {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of the TinyMCE assets are hardcoded to be loaded and not enqueued in a pluggable way. So we're creating a new class and requiring it conditionally, in which case it just doesn't load them anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this part? How does loading this class prevents TinyMCE from loading?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, by default, is a core WP class that takes care of generating and "enqueueing" the editor assets. The problem is that for some of the scripts it doesn't use the traditional way to enqueue scripts, rather it will just output a huge blob of them inline, see:

https://github.com/WordPress/wordpress-develop/blob/559e6cecf49505ecc3a21fbff647fdc5c3f892a3/src/wp-includes/class-wp-editor.php#L1567

So the only way to not spit that blob of scripts out is to override this class. This is actually an optimization because none of what it does will even get enqueued that way (look around the class I linked above, it also enqueued some assets), so we won't need to dequeue them either.

Luckily, redeclaring the class is possible because of the way that we check if the class is declared before being required, for example:

https://github.com/WordPress/wordpress-develop/blob/559e6cecf49505ecc3a21fbff647fdc5c3f892a3/src/wp-admin/includes/ajax-actions.php#L1923

and:

https://github.com/WordPress/wordpress-develop/blob/559e6cecf49505ecc3a21fbff647fdc5c3f892a3/src/wp-admin/includes/deprecated.php#L771

I realize this is a bold approach, but it makes it so straightforward to remove all editor assets that would otherwise be either enqueued or plainly spit out as inline scripts in the admin. This is one of the main reasons why I prefer having this whole thing wrapped as an experiment before releasing it to the public.

/**
* Necessary to ensure no additional TinyMcE assets are enqueued.
*/
public static function enqueue_default_editor() {}
}
86 changes: 86 additions & 0 deletions lib/experimental/disable-tinymce.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php
/**
* Experiment to disable TinyMCE and the Classic block.
*
* @package gutenberg
*/

// add_action( 'admin_footer', 'gutenberg_test_tinymce_access' ); // Uncomment the following line to force an external TinyMCE usage.
Copy link
Member Author

@tyxla tyxla May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncomment this line to force usage of TinyMCE. That can be used to test the cookie query argument logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to keep that for now, just in case we'd like to test 3rd party global usage.


/**
* Render a variable that we'll use to declare that the editor will need the classic block.
*/
function gutenberg_declare_classic_block_necessary() {
if ( ! gutenberg_post_being_edited_requires_classic_block() ) {
return;
}
echo '<script type="text/javascript">window.wp.needsClassicBlock = true;</script>';
}
add_action( 'admin_footer', 'gutenberg_declare_classic_block_necessary' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use an "editor" specific hook here like block_assets or thing like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because we need an action that actually is executed later and prepared to render an inline script. If we render during enqueue_block_assets, it's not guaranteed that window.wp will be declared.


// If user has already requested TinyMCE, we're ending the experiment.
if ( ! empty( $_GET['requiresTinymce'] ) || gutenberg_post_being_edited_requires_classic_block() ) {
return;
}


/**
* Disable TinyMCE by introducing a placeholder `_WP_Editors` class.
*/
function gutenberg_disable_tinymce() {
require __DIR__ . '/class--wp-editors.php';
}

add_action( 'admin_init', 'gutenberg_disable_tinymce' );

/**
* Enqueue TinyMCE proxy script.
* Detects TinyMCE usage and sets the `requiresTinymce` query argument to stop disabling TinyMCE loading.
*/
function gutenberg_enqueue_tinymce_proxy() {
wp_enqueue_script( 'gutenberg-tinymce-proxy', plugins_url( 'assets/tinymce-proxy.js', __FILE__ ) );
}

add_action( 'admin_enqueue_scripts', 'gutenberg_enqueue_tinymce_proxy' );

/**
* Example TinyMCE usage used for testing.
* Uncomment line 8 in this file to enable.
*/
function gutenberg_test_tinymce_access() {
echo '<script type="text/javascript">const a = window.tinymce.$;</script>';
}

/**
* Whether the current editor contains a classic block instance.
*
* @return bool True if the editor contains a classic block, false otherwse.
*/
function gutenberg_post_being_edited_requires_classic_block() {
if ( ! is_admin() ) {
return false;
}

// Handle the post editor.
if ( ! empty( $_GET['post'] ) && ! empty( $_GET['action'] ) && 'edit' === $_GET['action'] ) {
$current_post = get_post( intval( $_GET['post'] ) );
if ( ! $current_post || is_wp_error( $current_post ) ) {
return false;
}

$content = $current_post->post_content;
}

if ( empty( $content ) ) {
return false;
}

$parsed_blocks = parse_blocks( $content );
foreach ( $parsed_blocks as $block ) {
if ( empty( $block['blockName'] ) && strlen( trim( $block['innerHTML'] ) ) > 0 ) {
return true;
}
}

return false;
}
3 changes: 3 additions & 0 deletions lib/experimental/editor-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ function gutenberg_enable_experiments() {
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalInteractivityAPI = true', 'before' );
}

if ( gutenberg_is_experiment_enabled( 'gutenberg-no-tinymce' ) ) {
wp_add_inline_script( 'wp-block-library', 'window.__experimentalDisableTinymce = true', 'before' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the global variable that we use to pass the experiment to the client side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the thinking here? making this an experiment for some time to see the impact? What about removing the experiment but leaving this as a plugin-only (no backport to core) feature until we're certain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I prefer adding it as an experiment just in case, so we can further polish it without breaking the experience for anyone. I've also explained some of the rationale here. After I have more confidence, I'm happy to remove the experiment wrapper altogether.

}
}

add_action( 'admin_init', 'gutenberg_enable_experiments' );
12 changes: 12 additions & 0 deletions lib/experiments-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ function gutenberg_initialize_experiments_settings() {
)
);

add_settings_field(
'gutenberg-no-tinymce',
__( 'Disable TinyMCE and Classic block', 'gutenberg' ),
'gutenberg_display_experiment_field',
'gutenberg-experiments',
'gutenberg_experiments_section',
array(
'label' => __( 'Disable TinyMCE and Classic block', 'gutenberg' ),
'id' => 'gutenberg-no-tinymce',
)
);

register_setting(
'gutenberg-experiments',
'gutenberg-experiments'
Expand Down
6 changes: 5 additions & 1 deletion lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/experimental/navigation-theme-opt-in.php';
require __DIR__ . '/experimental/kses.php';
require __DIR__ . '/experimental/l10n.php';

if ( gutenberg_is_experiment_enabled( 'gutenberg-no-tinymce' ) ) {
require __DIR__ . '/experimental/disable-tinymce.php';
}
Comment on lines +112 to +114
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, this functionality is powered by a Gutenberg experiment. The experiment is supposed to work as a primary on/off button for the tinyMCE deprecation.


if ( gutenberg_is_experiment_enabled( 'gutenberg-interactivity-api-core-blocks' ) ) {
require __DIR__ . '/experimental/interactivity-api/blocks.php';
}
Expand All @@ -123,7 +128,6 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/experimental/interactivity-api/directives/wp-style.php';
require __DIR__ . '/experimental/interactivity-api/directives/wp-text.php';


// Fonts API.
if ( ! class_exists( 'WP_Fonts' ) ) {
// Fonts API files.
Expand Down
19 changes: 18 additions & 1 deletion packages/block-library/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ const getAllBlocks = () => {
buttons,
calendar,
categories,
...( window.wp && window.wp.oldEditor ? [ classic ] : [] ), // Only add the classic block in WP Context.
code,
column,
columns,
Expand Down Expand Up @@ -229,6 +228,24 @@ const getAllBlocks = () => {
queryTitle,
postAuthorBiography,
];

// When in a WordPress context, conditionally
// add the classic block and TinyMCE editor
// under any of the following conditions:
// - the current post contains a classic block
// - the experiment to disable TinyMCE isn't active.
// - a query argument specifies that TinyMCE should be loaded
if (
window?.wp?.oldEditor &&
( window?.wp?.needsClassicBlock ||
! window?.__experimentalDisableTinymce ||
!! new URLSearchParams( window?.location?.search ).get(
'requiresTinymce'
) )
) {
blocks.push( classic );
}

return blocks.filter( Boolean );
};

Expand Down
42 changes: 31 additions & 11 deletions packages/block-library/src/missing/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,34 +16,54 @@ import { safeHTML } from '@wordpress/dom';
function MissingBlockWarning( { attributes, convertToHTML, clientId } ) {
const { originalName, originalUndelimitedContent } = attributes;
const hasContent = !! originalUndelimitedContent;
const hasHTMLBlock = useSelect(
const { hasFreeformBlock, hasHTMLBlock } = useSelect(
( select ) => {
const { canInsertBlockType, getBlockRootClientId } =
select( blockEditorStore );

return canInsertBlockType(
'core/html',
getBlockRootClientId( clientId )
);
return {
hasFreeformBlock: canInsertBlockType(
'core/freeform',
getBlockRootClientId( clientId )
),
hasHTMLBlock: canInsertBlockType(
'core/html',
getBlockRootClientId( clientId )
),
};
},
[ clientId ]
);

const actions = [];
let messageHTML;
if ( hasContent && hasHTMLBlock ) {

const convertToHtmlButton = (
<Button key="convert" onClick={ convertToHTML } variant="primary">
{ __( 'Keep as HTML' ) }
</Button>
);

if ( hasContent && ! hasFreeformBlock && ! originalName ) {
if ( hasHTMLBlock ) {
messageHTML = __(
'It appears you are trying to use the deprecated Classic block. You can leave this block intact, convert its content to a Custom HTML block, or remove it entirely. Alternatively, you can refresh the page to use the Classic block.'
);
actions.push( convertToHtmlButton );
} else {
messageHTML = __(
'It appears you are trying to use the deprecated Classic block. You can leave this block intact, or remove it entirely. Alternatively, you can refresh the page to use the Classic block.'
);
}
} else if ( hasContent && hasHTMLBlock ) {
messageHTML = sprintf(
/* translators: %s: block name */
__(
'Your site doesn’t include support for the "%s" block. You can leave this block intact, convert its content to a Custom HTML block, or remove it entirely.'
),
originalName
);
actions.push(
<Button key="convert" onClick={ convertToHTML } variant="primary">
{ __( 'Keep as HTML' ) }
</Button>
);
actions.push( convertToHtmlButton );
} else {
messageHTML = sprintf(
/* translators: %s: block name */
Expand Down