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

Legacy widget: Text widget results in javascript error #33203

Closed
spacedmonkey opened this issue Jul 5, 2021 · 12 comments
Closed

Legacy widget: Text widget results in javascript error #33203

spacedmonkey opened this issue Jul 5, 2021 · 12 comments
Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Feature] Widgets Screen The block-based screen that replaced widgets.php.

Comments

@spacedmonkey
Copy link
Member

Description

The legacy text widget, use the tinymce error. If you have a text widget from the old widget editor then go to the new widget screen, this generates a javascript error.

TypeError: wp.editor.initialize is not a function
    at buildEditor (text-widgets.js?ver=5.7.2:265)
    at n.initializeEditor (text-widgets.js?ver=5.7.2:359)
    at renderWhenAnimationDone (text-widgets.js?ver=5.7.2:434)
    at HTMLDocument.handleWidgetAdded (text-widgets.js?ver=5.7.2:437)
    at HTMLDocument.dispatch (jquery.js?ver=3.5.1:5429)
    at HTMLDocument.elemData.handle (jquery.js?ver=3.5.1:5233)
    at Object.trigger (jquery.js?ver=3.5.1:8715)
    at HTMLDocument.<anonymous> (jquery.js?ver=3.5.1:8793)
    at Function.each (jquery.js?ver=3.5.1:381)
    at jQuery.fn.init.each (jquery.js?ver=3.5.1:203)

Step-by-step reproduction instructions

  1. Activate classic widgets plugins.
  2. Insert text widget
  3. Add some text to widget.
  4. Deactivate classic widgets plugins.
  5. Go to widget screen.
  6. See error.

Expected behaviour

No error message and the tinymce editor should be displayed.

Actual behaviour

Screenshots or screen recording (optional)

Screenshot 2021-07-05 at 17 12 36

Code snippet (optional)

WordPress information

  • WordPress version:
  • Gutenberg version:
  • Are all plugins except Gutenberg deactivated?
  • Are you using a default theme (e.g. Twenty Twenty-One)?

Device information

  • Device:
  • Operating system:
  • Browser:
@spacedmonkey spacedmonkey added [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets labels Jul 5, 2021
@draganescu
Copy link
Contributor

Testing this on the current 5.8 branch or WordPress master and I cannot repro, with or without Gutenberg active.
The problem appears on WordPress 5.7.2 and possibly earlier.

@spacedmonkey
Copy link
Member Author

I am seeing a similar error WP 5.9-alpha without the gutenberg plugin.

Screenshot 2021-07-05 at 22 08 42

@tellthemachines
Copy link
Contributor

I can reproduce this on WP 5.7.2 with the latest from this repo, but not on 5.9-alpha-51272-src with or without the Gutenberg plugin.

@noisysocks
Copy link
Member

Moved this to the post-5.8 grab bag ("Maybe later") as it's not affecting 5.8.

@kevin940726
Copy link
Member

I believe the problem is that the Text Widget is using trying to call wp.editor.initialize in text-widgets.js, where wp.editor is defined in base.js. However, Gutenberg also injects wp.editor global variable from the editor package, hence override the value.

Seems like we already have solved this before by reassigning it to wp.oldEditor. All we have to do is to change every occurrence of wp.editor to wp.oldEditor in text-widgets.js and the error should go away. And also the editor should display as expected:

image

@adamziel
Copy link
Contributor

adamziel commented Jul 6, 2021

Moved this to the post-5.8 grab bag ("Maybe later") as it's not affecting 5.8.

I think it is affecting 5.8.

There may be more problems here, but one originates in Jetpack when it is enabled in the latest WP 5.8 version, with or without the gutenberg plugin. Specifically, here:

wp_enqueue_script(
   'jetpack-blocks-editor',
   $editor_script,
   $editor_deps,
   $version,
   false
);

$editor_deps comes from plugins/jetpack/_inc/blocks/editor.asset.php which lists 26 different scripts including wp-edit-post and wp-editor. As soon as I comment them both out, everything works like a charm.

On the surface, this could be seen as a Jetpack problem so let me cc @jeherve and @kraftbj.

If we look deeper, however, this could be a common situation among many plugins and we should address it somehow. A somewhat naive solution could be to update text-widgets.js (and anything else) in core to first look for wp.oldEditor and only fall back to wp.editor if the former is not there. I said naive, because some plugins may rely on wp.editor and there isn't much we can do there. CCing @draganescu, @noisysocks, @getdave, @talldan – what do y'all think?

@adamziel
Copy link
Contributor

adamziel commented Jul 6, 2021

I just had the worst idea ever, but it could just solve this issue once and for all :D The problem is that we have two different sets of expectations in regard to what resides under the name wp.editor. Part of the code expects to find a module:

Zrzut ekranu 2021-07-6 o 17 59 31

While the rest of the code expects to find a legacy editor object with a few specific keys available:

Zrzut ekranu 2021-07-6 o 18 00 11

What if, instead of trying to match the right object with the right code, we just fuse the two together? So wp.editor would expose all the exported members of the module as well as all the members of wp.oldEditor?

@spacedmonkey
Copy link
Member Author

Sharing for visibility. A quick search on WordPress plugin directory for wp.editor.initialize brings up a lot of results. Not all of these will spin up tinymce in widget screen. But two popular ones seem too. SiteOrigin Widgets Bundle and Page Builder by SiteOrigin . SiteOrigin Widgets Bundle with 900k downloads. Not to mention Jetpack 5+ million downloads.

Worth testing against Jetpack and these plugins....

@adamziel
Copy link
Contributor

adamziel commented Jul 6, 2021

Great idea checking the plugin directory @spacedmonkey! This shows that we need a solution that's compatible with both the widgets editor and all the existing code. I just proposed one in #33228

@noisysocks
Copy link
Member

This is a related issue: https://core.trac.wordpress.org/ticket/53569

I wonder if we could add code to Core that warns against enqueueing wp-edit-post and wp-editor in the widgets screen?

@adamziel
Copy link
Contributor

adamziel commented Jul 7, 2021

Here's a PR proposing a very similar approach, but without touching the @wordpress/editor package: WordPress/wordpress-develop#1481
Also, there's another related issue: https://core.trac.wordpress.org/ticket/53437

@noisysocks
Copy link
Member

Let's go with WordPress/wordpress-develop#1481. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Feature] Widgets Screen The block-based screen that replaced widgets.php.
Projects
None yet
Development

No branches or pull requests

6 participants