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

TinyMCE: Use the same version used in Core #3840

Closed
wants to merge 1 commit into from

Conversation

youknowriad
Copy link
Contributor

This PR updates the TinyMCE script used in Gutenberg to match the TinyMCE used in the classic editor and other metaboxes. The Core version is slightly older (it probably has some bugs already fixed in newer versions) but this change fixes some errors we have right now due to using multiple TinyMCE versions:

I think if we were to update the TinyMCE version, we should find a way to do it globally.

closes #3512 #3731

@youknowriad youknowriad added [Feature] Meta Boxes A draggable box shown on the post editing screen [Component] TinyMCE labels Dec 7, 2017
@youknowriad youknowriad self-assigned this Dec 7, 2017
Copy link
Contributor

@ephox-mogran ephox-mogran left a comment

Choose a reason for hiding this comment

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

Just after confirmation that you are intending to still get the plugins from the fiddle azure website. There is an issue #3405 which deals with changing this ... so this seems like a good time to do it. Or is there a reason that you want to keep it?

'tinymce-latest',
'https://fiddle.azurewebsites.net/tinymce/' . $tinymce_version . '/tinymce' . $suffix . '.js'
);
$tinymce_version = '4.6.7';
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this is going lose a few of the recent fixes. Are you expecting to update before release?

'https://fiddle.azurewebsites.net/tinymce/' . $tinymce_version . '/tinymce' . $suffix . '.js'
);
$tinymce_version = '4.6.7';
wp_register_script( 'tinymce-latest', includes_url( 'js/tinymce/' ) . 'wp-tinymce.php', array( 'jquery' ), false, true );
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason that you are still getting the plugins from the fiddle below?

@youknowriad
Copy link
Contributor Author

so this seems like a good time to do it. Or is there a reason that you want to keep it?

No reason honestly, I was just trying to tackle something else (aligning TinyMCE versions).

Yeah, this is going lose a few of the recent fixes. Are you expecting to update before release?

The problem is, I don't know how to update the Core's version without touching Core. If you have some ideas here, that would be great.

@ellatrix
Copy link
Member

I don’t really feel comfortable reverting to an older version of TinyMCE as there were a lot of fixes that affect Gutenberg in recent versions. Is there a way to remove the original script and re-enqueue it with the latest version? I’m guessing that the script in added without this api in core, and now assumed there by Gutenberg? How do we make sure the dependency is explicit?

@youknowriad
Copy link
Contributor Author

Is there a way to remove the original script and re-enqueue it with the latest version?

I don't know :P you have probably more experience with TinyMCE in Core than I have. I'll see if I can find how the script is enqueued in Core and plugins.

I’m guessing that the script in added without this api in core, and now assumed there by Gutenberg? How do we make sure the dependency is explicit?

I'm not sure I understand, I'm still registering the tinymce-latest script in Gutenberg and adding a dependency to it, so it's still an explicit dependency. I changed just the source of the registered script.

@ellatrix
Copy link
Member

Oh, I looked over wp_register_script( 'tinymce-latest' ... :) With this line though, we'll be loading TinyMCE twice?

@davisshaver
Copy link
Contributor

This pull request fixes the location errors with TinyMCE but it does not solve the larger problem quite yet, as in my testing the wp_editor() field will parse null in the request to post.php?post=xxxx&action=edit&classic-editor=1&meta_box=1 with metabox values.

@davisshaver
Copy link
Contributor

so we need to update TinyMCE field before we send the action. We can do this with:

if ( window.tinyMCE ) {
    window.tinyMCE.triggerSave();
}

I am looking for an appropriate spot to place this

@youknowriad
Copy link
Contributor Author

Closing the PR for now, Let's fix that in Core first by changing the way we load TinyMCE in the classic editor and wp_editor

@youknowriad youknowriad deleted the update/use-core-tinymcee-version branch April 20, 2018 13:21
@danielbachhuber
Copy link
Member

Closing the PR for now, Let's fix that in Core first by changing the way we load TinyMCE in the classic editor and wp_editor

@youknowriad @iseulde Is there a Trac ticket for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Meta Boxes A draggable box shown on the post editing screen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Classic Text block fails to preview if ACF is installed
5 participants