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

Fix the TinyMCE init array #10968

Merged
merged 5 commits into from Nov 20, 2018

Conversation

Projects
None yet
5 participants
@azaozz
Contributor

azaozz commented Oct 23, 2018

closes #10966, #11147, #7942, #8670

  • Added "manual" convert from php to js, same as in class-wp-editor.php.
  • Reformatted the code there so it's more readable.
Fix the TinyMCE init array
- Added "manual" convert from php to js, same as in class-wp-editor.php.
- Reformated the code so it's more readable.

@azaozz azaozz requested review from aduth and iseulde Oct 23, 2018

'wptextpattern',
'wpview',
);
$tinymce_plugins = apply_filters( 'tiny_mce_plugins', $tinymce_plugins, 'classic-block' );

This comment has been minimized.

@iseulde

iseulde Oct 24, 2018

Member

Why shouldn't we pass editor if we want it to be as close as possible like the old editor?

This comment has been minimized.

@azaozz

azaozz Oct 24, 2018

Contributor

That is (supposedly) the editor id and is intended to identify the TinyMCE instance when more than one on the same page. The classic editor passes content there, but thinking it's no good to reuse that as the classic block editor instance is quite different and plugins should be able to identify it from php.

Passing editor is a bit too generic :)

@iseulde

This comment has been minimized.

Member

iseulde commented Oct 24, 2018

Added "manual" convert from php to js, same as in class-wp-editor.php.

Is it not possible to compose the right PHP object, then JSON encode?

@azaozz

This comment has been minimized.

Contributor

azaozz commented Oct 24, 2018

No, because that breaks some of the functionality. The TinyMCE init is supposed to be written in (pure) js. It is in our (WordPress) implementation that we set it as php and then "convert" to js before output.

Several settings in the init object accept js functions. If it is JSON encoded, that breaks. Example: init_instance_callback. To test can use the mini WP plugin from here: #10966 (comment).

Currently when using that plugin we get this in the console:

Uncaught TypeError: Cannot read property 'apply' of undefined
    at Editor.execCallback (tinymce.js?ver=4800-20180716)
    at initEditor (tinymce.js?ver=4800-20180716)
    .....

Happens because the init js is outputted by wp_localize_script() which is meant only for strings and JSON encodes the output.

With this pr we get:

Editor: editor-7ffd9aaf-ae66-47f1-a160-1820fe202478 is now initialized.

i.e. works as expected.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Oct 24, 2018

Also, this pr adds another (custom) option to the TinyMCE init object: 'classic_block_editor' => true,. It would be nice to be able to identify the TinyMCE instance from js based plugins. Will add similar option to the Classic editor's instance, probably 'content_editor' => true, (or maybe that should be classic_editor?).

@azaozz

This comment has been minimized.

Contributor

azaozz commented Oct 24, 2018

Heh? Tests failing because... the code is more readable? :)

     |         |     expected 13 space(s) between "'toolbar1'" and
     |         |     double arrow, but found 1.

This seems contrary to the WP coding standards. So when defining arrays they should look like:

'long-array-key-that-would-make-for-a-long-line-and-there-is-no-way-to-wrap-it-up' => 'value',
'key'                                                                              => 'long-array-key-value-that-is-not-easy-to-read-as-it-makes-the-line-too-long',

The spacing looks so "borked" :) How is that easy to read? :)

Opened WordPress-Coding-Standards/WordPress-Coding-Standards#1513 as follow up.

@danielbachhuber

This comment has been minimized.

Member

danielbachhuber commented Oct 30, 2018

Note: this doesn't quite yet solve #11147 (comment)

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 2, 2018

Also opened https://core.trac.wordpress.org/ticket/45221 for core.

This is a regression and a blocker for WP 5.0.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 8, 2018

Fixed in core https://core.trac.wordpress.org/changeset/43867. The fix needs to be merged here too.

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 20, 2018

Is this ready to land? Can we rebase and merge @iseulde @azaozz ?

azaozz and others added some commits Nov 20, 2018

@youknowriad

LGTM 👍

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 20, 2018

These changes are already in Core, so no need to backport.

@youknowriad youknowriad added this to the 4.5 milestone Nov 20, 2018

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 20, 2018

Ughhh, PHPCS forcing ugly code formatting :(
(I should have fixed that, sorry).

@youknowriad youknowriad merged commit 18d0fbc into master Nov 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/tinymce-init-array branch Nov 20, 2018

daniloercoli added a commit that referenced this pull request Nov 20, 2018

Merge branch 'master' of https://github.com/WordPress/gutenberg into …
…rnmobile/wire-on-replace-para-block

* 'master' of https://github.com/WordPress/gutenberg:
  Remove "permalink settings" link from permalink panel. (#12121)
  preserve quote content (#12122)
  Replace gutenberg domain with default for Core blocks (#12108)
  Fix issue with disabled togglecontrol double border (#12091)
  Fix the TinyMCE init array (#10968)

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Fix the TinyMCE init array (WordPress#10968)
* Fix the TinyMCE init array

- Added "manual" convert from php to js, same as in class-wp-editor.php.
- Reformated the code so it's more readable.

* Fix missing space to fix failing tests...

* Fix phpcs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment