Opened 15 months ago

Closed 15 months ago

Last modified 15 months ago

#20055 closed enhancement (fixed)

Load .dev.js scripts for TinyMCE plugins if $concatenate_scripts == false

Reported by: ericlewis Owned by: azaozz
Priority: normal Milestone: 3.4
Component: TinyMCE Version:
Severity: normal Keywords:
Cc:

Description

After banging my head on a Javascript / TinyMCE wall for a bit this morning, I figured I would write a patch for this issue. This makes debugging TinyMCE plugins a lot easier.

If $concatenate_scripts is false ( which is automatically FALSE if SCRIPT_DEBUG is true ) editor_plugin.dev.js files will be scanned for and included automatically, and if they don't exist their editor_plugin.js counterpart is loaded.

I spoke to _duck in IRC and he implied there may be a reason for not loading .dev.js scripts, so there may be a reason not to do this, however I can't really see one.

Attachments (6)

1.diff (1.3 KB) - added by ericlewis 15 months ago.
20055.diff (148.0 KB) - added by ericlewis 15 months ago.
packaged .dev.js non-WP plugin files from TinyMCE source; also themes/advanced/editor_template.dev.js added ternary declaration of suffix of tinyMCEPreInit depending on SCRIPT_DEBUG constant.
20055.1.diff (154.0 KB) - added by ericlewis 15 months ago.
As per Nacin's suggestion, remove script_concat_settings(), move logic for compressions and concatenations to the function calls. Still has respect to the globals in case any plugin sets them. Just saw azaozz's comment, this will need to be ammended to use MCE debug mode.
test_compression.diff (2.8 KB) - added by nacin 15 months ago.
20055-2.patch (213.9 KB) - added by azaozz 15 months ago.
20055-3.patch (7.0 KB) - added by ericlewis 15 months ago.
If this is still on the table, this patch fixes the "polluted globals" issue Nacin cited earlier, using a new canonical format, which respects the global then the constant, varying a bit between the different cases. Also implements the "_src" conditional loading azaozz implemented earlier. I'll learn how to create a diff from a patch and more changes for next time ;)

Download all attachments as: .zip

Change History (19)

comment:1 follow-up: ↓ 3   nacin15 months ago

Not sure what the best solution here is, but something needs to happen here to clean this up.

The script-loader a while back got polluted with globals $concatenate_scripts, $compress_scripts, and $compress_css. This is silly as we also have constants for this, and also a site option called 'can_compress_scripts'. None of these are canonical. It's silly to have all of these settings. script_concat_settings() should die and the values should be wrapped up into a function or two. Then TinyMCE should also respect SCRIPT_DEBUG.

For this particular problem:

Hypothetically, setting tinyMCEPreInit.suffix to be '.dev' is enough to load editor_plugin.dev.js files. But, if the .dev.js file does not exist, as it does for all non-WP plugins, then TinyMCE breaks. I would have no problems packaging the .dev.js files for the non-WP plugins if that's what it takes. It doesn't look like external plugins (added by WP plugins) would be affected.

packaged .dev.js non-WP plugin files from TinyMCE source; also themes/advanced/editor_template.dev.js added ternary declaration of suffix of tinyMCEPreInit depending on SCRIPT_DEBUG constant.

20055.diff fixes the issue as Nacin suggested, but doesn't deal with the polluted globals issue.

comment:3 in reply to: ↑ 1   azaozz15 months ago

Replying to nacin:

The script-loader a while back got polluted with globals $concatenate_scripts, $compress_scripts, and $compress_css. This is silly as we also have constants for this, and also a site option called 'can_compress_scripts'. None of these are canonical. It's silly to have all of these settings.

Agreed. Now most of these aren't needed. At the time of adding them few years ago, some web servers had different issues when compressing scripts from PHP (disregarding headers, double compressing, etc.) that seem (mostly?) resolved now.

Hypothetically, setting tinyMCEPreInit.suffix to be '.dev' is enough to load editor_plugin.dev.js files.

Right, the problem here is mostly with MCE plugins added by WP plugins not having .dev.js files and also with the default MCE plugins having slightly different naming scheme (_src.js vs. .dev.js).

The other part is that MCE plugins are not loaded through script_loader. They are loaded by MCE using non-blocking methods. So force-loading everything as in 1.diff above would not work well. MCE has a debug mode loading/init we should use for that. The only requirement would be to rename our custom plugins that are in the MCE's plugins directory from .dev.js to _src.js. There doesn't seem to be a problem with that as currently the .dev.js files there are not used anywhere (they are distributed only as sources for the editor_plugin.js files).

As per Nacin's suggestion, remove script_concat_settings(), move logic for compressions and concatenations to the function calls. Still has respect to the globals in case any plugin sets them. Just saw azaozz's comment, this will need to be ammended to use MCE debug mode.

The only requirement would be to rename our custom plugins that are in the MCE's plugins directory from .dev.js to _src.js. There doesn't seem to be a problem with that as currently the .dev.js files there are not used anywhere (they are distributed only as sources for the editor_plugin.js files).

Let's do it. bumpbot can easily account for the change.

nacin15 months ago

test_compression.diff is the directionality plugin's editor_plugin.js file. Looks like the minification is the same technique, but variables are assigned differently.

We will A) need to blacklist external plugins for bumpbot, B) whitelist WP plugins for bumpbot, or C) let bumpbot compress the external editor_plugin.js files the way it wishes. C, of course, is easiest.

I also forgot that wp-tinymce.js.gz needs to be built. Thinking bumpbot.php should do this as well.

Replying to nacin:

We will A) need to blacklist external plugins for bumpbot, B) whitelist WP plugins for bumpbot, or C) let bumpbot compress the external editor_plugin.js files the way it wishes. C, of course, is easiest.

As the number of our custom plugins for MCE doesn't change a lot (4 times since WP 2.5) and we shouldn't minify the default MCE plugins that come in the MCE distribution, thinking it would be better to whitelist our plugins in bumpbot.

Alternatively we can whitelist all plugins whose directory name starts with "wp" or "wordpress".

Yes, wp-tinymce.js.gz can be build automatically. The format is pretty loose: the MCE's core has to go in first and the "mark as loaded" function has to be last. Not a big priority though as for development it can be re-added by hand after changing one of our MCE plugins.

20055-2.patch seems to work properly. The only concern is about WP plugins that add MCE plugins and don't follow the "right way to do it" as described at 'mce_external_plugins' filter.

Version 0, edited 15 months ago by azaozz (next)

If this is still on the table, this patch fixes the "polluted globals" issue Nacin cited earlier, using a new canonical format, which respects the global then the constant, varying a bit between the different cases. Also implements the "_src" conditional loading azaozz implemented earlier. I'll learn how to create a diff from a patch and more changes for next time ;)

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

In [19945]:

Load TinyMCE's non-minified "*src.js" plugin files when SCRIPT_DEBUG is defined, part props ericlewis, fixes #20055

This commit also includes the source for the mark_loaded function that goes at the bottom of wp-tinymce.js.gz: tinymce/mark_loaded_src.js.

Last edited 15 months ago by azaozz (previous) (diff)
  • Milestone changed from Awaiting Review to 3.4
  • Resolution fixed deleted
  • Status changed from closed to reopened

I think we should reconsider the implementation here. I'll comment in a bit.

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

Never mind. :-)

Will continue charset discussions in #19592.

script_concat_settings() continued in #20066. bumpbot aspects were handled in #19592.

Note: See TracTickets for help on using tickets.