Make WordPress Core

Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#16284 closed enhancement (fixed)

Remove the wpdialog plugin from TinyMCE

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 3.9 Priority: high
Severity: normal Version: 3.1
Component: Editor Keywords: close
Focuses: Cc:

Description

Combining TinyMCE with jQueryUI is not a very good idea IMHO:

  • TinyMCE has all the functionality to create dialogs which is native, more extended and seems more robust than jQuery UI.
  • The reason why most dialog type scripts use an iframe is to remove any possibility of conflicts with the parent page. In out case such conflict might be introduced by a plugin that alters the edit screen.
  • There were suggestions some time ago to stop using jQuery UI in WordPress admin and replace it with one of the other jQuery plugins with similar functionality that are a lot more concise and robust.

Attachments (3)

16284.patch (1004 bytes) - added by azaozz 9 years ago.
16284.1.patch (1.5 KB) - added by azaozz 9 years ago.
16284.2.patch (1.3 KB) - added by azaozz 9 years ago.

Download all attachments as: .zip

Change History (23)

#1 @ocean90
9 years ago

  • Keywords close added
  • Milestone changed from Future Release to 3.9

Related: [26876], [26899], #24067

I think we can close this in favour of #24067.

#2 @azaozz
9 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Right, fixed in [26899]. TinyMCE 4.0 supports inline dialogs natively.

#3 @harmr
9 years ago

Are there any tutorial available what to use instead? my plugin adds a button which opens a search window for maps. Till now this has be done with wpdialog(). Now I tried to change this to

$(document).on('click', '#upload-new-icon', function(){
     tinyMCE.get()[0].windowManager.open({
           title: "<?php esc_attr_e('Upload new icon','lmm'); ?>",
           url: "<?php echo LEAFLET_PLUGIN_URL . 'inc/icon-upload.php?_wpnonceicon=' . $noncelink_uploadicon; ?>",
           width: 450,
           height: 120,
           inline: 1,
     });
)};

which works fine if visual editor is used. Anyway if visual editor is disabled, this approach fails.
Any help would be really appreciated!
thx,
Robert

#4 @azaozz
9 years ago

The purpose of 'wpdialog' was to make it possible to use the native TinyMCE 3.x dialogs API with inline HTML (it supports only iframes). In TinyMCE 4.0 that API was updated to support inline and dynamically created HTML, so the functionality of the 'wpdialog' plugin is no longer needed.

Generally you can use any code to create a modal dialog. If you want your modals to be draggable and resizable (but not responsive) you can continue using UI dialog. There is no requirement to use the TinyMCE API for creating modals. This is very handy when you want your modal to be available in both the Visual and Text editors.

The button added to the TinyMCE toolbar can call any JS and you can pass the editor instance (or just the editor ID) and use the TinyMCE APIs there. Have a look at the current wplink.js.

#5 @harmr
9 years ago

thanks for the explanation - I think I will load my content via tb_show(), that should be the approach I think...

#6 @Ipstenu
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Removing wpdialogs also removes jquery-ui-dialog, which has the potential to turf any plugin that assumed its existence. I'm not sure how we might search for it though. Obviously them enquing properly in the plugin would be the right fix, but without a way to grep how many people we're going to impact, I don't know how to fully estimate this possible impact.

From a users perspective, it's a "I clicked on your button in the visual editor, and nothing happened!"

A dev may not even realize that the UI is why it's not working at all until they check the logs, because they may not have realized they were using it.

https://wordpress.org/support/topic/jquery-ui-dialog-script-moved-out-of-39-latest-build?replies=9&view=all

#7 @iseulde
9 years ago

This was removed in [27494]. Note this script was only enqueued when there is a link button in the toolbar, so if another plugin removed the link button, the plugin that doesn't add dependencies properly would also break pre-3.9. I'd say wontfix unless we add this script unconditionally to the editor (even though it wasn't before). Still, the script is then only added to the editor, not the page. So if this plugin is supposed to work for custom post types without the editor, it will break again.

#8 @iseulde
9 years ago

Pre 3.9:

if ( in_array('wplink', self::$plugins, true) || in_array('link', self::$qt_buttons, true) ) { 
    wp_enqueue_script('wplink'); 
    wp_enqueue_script('wpdialogs'); 
    wp_enqueue_style('wp-jquery-ui-dialog'); 
}

#9 @nacin
9 years ago

We'll be discussing this at the next dev chat (April 2).

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


9 years ago

#11 @nacin
9 years ago

IRC consensus was to leave this as is for now; to investigate how often plugins might be using these APIs; and document it on make/core.

#12 @azaozz
9 years ago

  • Milestone 3.9 deleted
  • Resolution set to worksforme
  • Status changed from reopened to closed

Yes, the dev. chat consensus was to not try to bail out plugins that don't properly set dependencies for their scripts.

The actual wpdialogs TinyMCE plugin still exists and is loaded for back-compat. For plugins to continue to use it (which is pointless in TinyMCE 4.0), they have to make sure their JS is dependent on wpdialog.js (this will load all components of UI Dialog).

@azaozz
9 years ago

#13 @azaozz
9 years ago

Actually better to show an error in the console when wpdialog.js is not loaded and a plugin tries to use the 'wpdialog' TinyMCE plugin.

@azaozz
9 years ago

#14 @azaozz
9 years ago

  • Milestone set to 3.9
  • Resolution worksforme deleted
  • Status changed from closed to reopened

In 16284.1.patch: also add an action when enqueueing the scripts and styles for external editor components (like wpLink, wp-fullscreen, etc. This will let plugins enqueue wpdialogs and/or their own scripts that may be needed when wp_editor() is used. The action would also be convenient for enqueueing/registering wpViews.

Reopening for consideration.

This ticket was mentioned in IRC in #wordpress-dev by azaozz. View the logs.


9 years ago

#16 @azaozz
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 28012:

TinyMCE: show an error in the console when a plugin attempts to use the 'wpdialogs' plugin but wpdialog.js is not enqueued, fixes #16284

#17 @nacin
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for final fixes, per IRC:

  • Typo in [28012] comment (wpdialog.js)
  • Adding a wp_enqueue_editor hook, with a single array of boolean contexts (not multiple arguments)
  • Specifying the script handle (rather than / in addition to the filename) in the console error

@azaozz
9 years ago

#18 @azaozz
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 28024:

Fix typo and add more text to the error shown when 'wpdialogs' is not enqueued. Add 'wp_enqueue_editor' action fired when scripts and styles for the editor are being enqueued, fixes #16284

#19 @DrewAPicture
9 years ago

In 28026:

Improve hook docs for the wp_enqueue_editor hook, added in 3.9.

See #16284, #27700.

#20 @azaozz
9 years ago

In 28045:

Precommit cleanup for theme.js, editor.css amd wpdialogs/plugin.js, See #27055, #26907, #16284

Note: See TracTickets for help on using tickets.