Make WordPress Core

Opened 5 years ago

Closed 4 months ago

#49964 closed enhancement (maybelater)

Support asynchronously loading TinyMCE

Reported by: sarayourfriend's profile sarayourfriend Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0
Component: TinyMCE Keywords: has-patch needs-testing 2nd-opinion
Focuses: administration, performance Cc:

Description

In order to facilitate asynchronously loading TinyMCE in Gutenberg we need to be able to prevent WordPress from automatically enqueueing wp-tinymce and injecting inline i18n initialization scripts. (There's plenty of context in the Gutenberg issue including related tickets, so I'll try not to repeat any of that here.)

I'd like to propose wrapping _WP_Editors#print_tinymce_scripts in an action that Gutenberg could use for when the editor is loading.

Change History (14)

#1 @sarayourfriend
5 years ago

FWIW, the title of this ticket might be better off something like "Add a way to prevent TinyMCE scripts from being injected". Wrapping the existing logic seemed like the most straightforward way to let Gutenberg override the existing behavior to prevent the scripts from loading alongside it.

This ticket was mentioned in PR #232 on WordPress/wordpress-develop by saramarcondes.


5 years ago
#2

This is a draft PR to start addressing the issue in the ticket. Moving this logic into an action allows plugins to override how/when TinyMCE is loaded. Specifically, Gutenberg needs to be able to prevent TinyMCE scripts from being written into the page when TinyMCE should be loaded asynchronously when it is actually needed.

This is the first time I've made changes in WordPress core and I had a hard time deciding where to put the wp_print_tinymce_scripts function (or what to name it, for that matter).

I'm also not even sure that this is exactly the correct approach for the issue described in the ticket, so any and all feedback is super welcome!

Trac ticket: https://core.trac.wordpress.org/ticket/49964

#3 @azaozz
5 years ago

  • Milestone changed from Awaiting Review to 5.5

Welcome to Trac and thanks for the ticket and the patch!

This sounds good but perhaps can be expanded a bit to look into the loading/outputting of all of the TinyMCE initialization/dependency scripts. Perhaps TinyMCE should be able to be enqueued through script-loader like any other script in WP, and all of the dependencies handled through there. Then it would become possible to "get" the dependencies through the REST API after #48654 and https://github.com/WordPress/gutenberg/pull/21244 are ready/committed.

Setting tentatively to 5.5 depending on progress of the above two tickets.

Last edited 5 years ago by azaozz (previous) (diff)

#4 @sarayourfriend
4 years ago

  • Summary changed from Wrap _WP_Editors#print_tinymce_scripts in a hook to Support asynchronously loading TinyMCE

@azaozz do you think it would be possible to use this ticket just for the async TinyMCE work, meaning the minimum lift required for that performance win for Gutenberg? I'm happy to open another trac ticket with extra details about tech debt around TinyMCE and how we could go about getting the three inline scripts related to bootstrapping TinyMCE into a single place that the REST API you linked would be able to use.

In the mean time, I've expanded the patch to include an addition to WP_Scripts, a new method, get_url. This is a copy of the method from the PR in Gutenberg @spacedmonkey has open for the REST API. I've added tests and moved it into core so that it can be more transparently used for a given handle.

On the other hand, it occurred to me after adding it to core that it might do just as well to add it as a utility in Gutenberg. Let me know what the preference is and I'll make the change if necessary. It's needed outside the REST API for this PR: https://github.com/WordPress/gutenberg/pull/23068/files#diff-26f7754641e020003bf92e16ac862f50R199

#5 @spacedmonkey
4 years ago

  • Keywords needs-patch added
  • Version set to 5.0

I think the efforts on this ticket should be moved to #48654, which has a very similar goal. I would wait to see what happens with that ticket, before continuing work on this ticket.

#6 @azaozz
4 years ago

do you think it would be possible to use this ticket just for the async TinyMCE work

Sure, lets keep the original scope. I've looked a bit at what it will take to fully support enqueueing of TinyMCE through script loader, and it seems it needs some more substantial changes (directly adding all of the TinyMCE dependencies to script-loader doesn't make sense as that will load quite a bit of pointless/unused inline js on every run and fire several pointless/unused actions and filters).

I think the efforts on this ticket should be moved to #48654, which has a very similar goal.

Right, the goal is similar but #48654 deals with scripts and stylesheets enqueued through script-loader. TinyMCE is an exception and its dependencies are generated and outputted differently, hence the need of this ticket.

#7 @azaozz
4 years ago

  • Keywords has-patch needs-testing 2nd-opinion added; needs-patch removed

#8 follow-up: @davidbaumwald
4 years ago

@azaozz Do you feel this can still make the deadline before Beta 1 for 5.5 tomorrow, or should this be moved out of the milestone?

#9 @davidbaumwald
4 years ago

  • Milestone changed from 5.5 to Future Release

With Beta 1 for 5.5 landing in just a few hours, this is being moved to Future Release. If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#10 in reply to: ↑ 8 @azaozz
4 years ago

Replying to davidbaumwald:

Do you feel this can still make the deadline before Beta 1 for 5.5 tomorrow

Yeah, enqueueing all of the TinyMCE parts through script loader turned out to be more difficult/challenging that expected. Lets try it in 5.6.

#11 @sarayourfriend
4 years ago

Now that 5.5 is out the door, do you @azaozz or @spacedmonkey have any thoughts regarding whether we can get this project moving forward again?

#12 @spacedmonkey
4 years ago

@sarayourfriend I think this is blocked by https://core.trac.wordpress.org/ticket/48654 . I tried to get it in 5.5, but @azaozz didn't like the idea of replicating scripts and styles system in REST API. Not sure where we are with that ticket. Until we get some focus, this ticket can not continue.

#13 @sarayourfriend
4 years ago

@spacedmonkey I think that blocks this corresponding GitHub issue: https://github.com/WordPress/gutenberg/issues/2768

But the TinyMCE solution that @azaozz and I eventually came up with doesn't use the REST API, opting instead for the existing mechanisms: https://github.com/WordPress/gutenberg/pull/23068/files#diff-26f7754641e020003bf92e16ac862f50R267-R276

We did this specifically so as not to be blocked by the REST API 😊

The last thing I heard about this I think was that @azaozz had realized some issues with translations for TinyMCE plugins that needed to be addressed, but it may have fallen off in the midst of the 5.5 release.

@azaozz Do you happen to have any notes about that I could use to try to sus out a solution? Even just a plugin you know that would break on the current PR would be helpful ☺️

#14 @azaozz
4 months ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

This is still a good idea imho. Please feel free to reopen if/when the corresponding Gutenberg issue is reopened.

Note: See TracTickets for help on using tickets.