-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
TinyMCE use the Core TinyMCE version to avoid conflicts #6848
Conversation
1699e93
to
77fdc7e
Compare
Should we require WordPress 4.9.6 with this change? |
Hm, not sure if we have a list of all the bugs it will cause. I think it'd be good to avoid a bunch of reports caused merely by an older WordPress version. |
lib/client-assets.php
Outdated
@@ -76,6 +76,8 @@ function gutenberg_register_scripts_and_styles() { | |||
gutenberg_register_vendor_scripts(); | |||
|
|||
// WordPress packages. | |||
wp_register_script( 'wp-tinymce', includes_url( 'js/tinymce/' ) . 'wp-tinymce.php', array( 'jquery' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does anything break without the jQuery dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know :) Probably not. It started as a copy/paste from my old PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's required for some custom WordPress plugins, but we're not loading those.
'tinymce-latest', | ||
'https://unpkg.com/tinymce@' . $tinymce_version . '/tinymce' . $suffix . '.js' | ||
); | ||
$tinymce_version = '4.7.11'; | ||
gutenberg_register_vendor_script( | ||
'tinymce-latest-lists', | ||
'https://unpkg.com/tinymce@' . $tinymce_version . '/plugins/lists/plugin' . $suffix . '.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could load these from core too, except the table plugin. Maybe better like this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought they were loaded automatically so I removed this but it didn't work. I believe we could use the Core provided once but it's not as urgent as TinyMCE.
'tinymce-latest', | ||
'https://unpkg.com/tinymce@' . $tinymce_version . '/tinymce' . $suffix . '.js' | ||
); | ||
$tinymce_version = '4.7.11'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh too bad we can't base that on https://github.com/WordPress/wordpress-develop/blob/186bb7c8a908de977934d77fc5633b493ba2223f/src/wp-includes/version.php#L21
Testing with: add_action( 'add_meta_boxes', function() {
add_meta_box( 'wp_editor_test_box_1', 'wp_editor() test box', function( $post ) {
wp_editor( $post->post_content, 'wp_editor_test_1' );
} );
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me! Just wondering if we should maybe load an unminified version for script debug mode.
It's fine to bump the version. But maybe let's wait until 3.0 for doing the bump. |
@youknowriad Thanks a lot for the merge! |
This PR uses the TinyMCE core scripts instead of using TinyMCE from the CDN to avoid conflicts between the classic editor/wysiwyg metaboxes and the Gutenberg editor.
We're still loading the TinyMCE script two times because there's no registered script for TinyMCE in Core but at least this fixes the compatibility issues.
closes #6809 #6847 #3302 #3731