The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.#841794: Cache wysiwyg_load_includes() was committed to the Wysiwyg 7.x-2.x branch a while ago to improve performance a bit, but it had the side effect of breaking your Wysiwyg plugin. Wysiwyg is no longer letting it load the additional script files it needs when the plugin metadata is loaded from the cache. We'll most likely revert that change in Wysiwyg for now, but the patch will eventually get in.
To make cached metadata usable, we first need to test and finalize the patch from #1039076: Allow plugins to specify libraries and multiple javascript files, so your plugin code can include all the files it needs in the metadata itself. I've already attached a patch for Media to that issue. It will use proposed metadata properties instead of direct drupal_add_js/css calls from within the plugin file. The new metadata property is currently called 'attached files' and works exactly the same as the '#attached' property from FAPI, so I hope it should cover all your needs. See the issue for details.
If you could review the patches from your perspective, it would be greatly appreciated.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | media-attached-files.1795968.5.patch | 1.9 KB | twod |
| #1 | media-attached-files.1039076.1.patch | 2.33 KB | devin carlson |
Comments
Comment #1
devin carlson commentedI follow the latest Media and WYSIWYG -dev versions and ran into this issue (I ended up reverting #841794: Cache wysiwyg_load_includes()).
The patch at #1039076: Allow plugins to specify libraries and multiple javascript files looks good and solves the issue of the media browser failing to load. I've attached a re-roll with the following changes:
Trailing whitespace.
You called the new property
attached files. :PComment #2
twodLooks like I uploaded the wrong file, sorry about that and thanks for fixing it.
#841794: Cache wysiwyg_load_includes() has now been reverted in Wysiwyg and we'll wait until the multiple files issues are committed to re-commit it.
Comment #3
ParisLiakos commentedso, i am pretty confused..what are we doing here?
if i commit this patch will it break anything?
Comment #4
twod@rootatwc, once #1039076: Allow plugins to specify libraries and multiple javascript files gets into Wysiwyg, the patch in #1 can be committed to Media. After that, we can put #1039076: Allow plugins to specify libraries and multiple javascript files back into Wysiwyg without breaking Media's plugin.
If you commit the patch #1 now, as-is and before #1039076: Allow plugins to specify libraries and multiple javascript files is in, the additional scripts needed by Media's plugin won't get loaded at all because Wysiwyg doesn't read the 'attached files' property yet.
If we don't want to have to time our commits to minimize the time the modules are out of sync, maybe we could create a hybrid-patch for Media? It might be possible to make a new Media patch which adds the script files both using direct
drupal_add_js()calls AND the 'attached files' property and commit that now. I've not tried it but since Drupal doesn't add the same file more than once, it should work.Essentially, keep
media_include_browser_js()but move themodule_load_include()call out to where it is in the #1 patch and add the same'attached files' => media_browser_js(),line.That should make sure that Media's plugin works both before and after we enable caching of
wysiwyg_load_includes()because we must commit #1039076: Allow plugins to specify libraries and multiple javascript files first.If that's necessary, I'll leave up to you, all I can promise is that #1039076: Allow plugins to specify libraries and multiple javascript files just before #841794: Cache wysiwyg_load_includes() in the same [next] stable release.
Comment #5
ParisLiakos commentedLets do this.
And then when next wysiwyg version is released we remove the cruft
Comment #6
twodI've re-added the old code and merged in the new code from #1.
Note that a new version of the Wysiwyg patch is needed so that your settings aren't added twice (I completely overlooked the last drupal_add_js() call in the original patch). See #1039076-7: Allow plugins to specify libraries and multiple javascript files.
This way Media can be compatible both with older and newer versions of Wysiwyg without either of us having to worry about timing the release.
I'll be waiting a short while for a response from Sun on the other issue just in case he's got a very strong opinion against this approach.
Comment #7
dnotes commentedThis is all a little hard to follow - am I correct in assuming that this set of issues is likely to be currently causing problems like #1866284: Videos in node body do not show up at all on page view and #1950636: It shows me only images but not the video iframe?
What I'm seeing is that media_vimeo_preprocess_media_vimeo_video() is not running on node view pages, and therefore the javascript (and settings) are not loaded, resulting in the
tags that get cached in cache_field are not showing up nor being converted to iframes. I'm using- Wysiwyg 2.2
- Media 1.3
- Media Vimeo 1.0-beta5
- Media Youtube 1.0-beta3
If my suspicions are correct, then are there plans to backport the fixes to Media 1.x as well? I don't really feel comfortable reviewing this because I don't yet use 2.x.Comment #8
twod@dnotes, I doubt this is what's causing those issues since none of the code related to this caching optimization has been added to Wysiwyg or Media yet. A patch was committed to the Wysiwg -dev snapshots a while ago but got reverted soon after when it was discovered Media - and possibly other modules with Wysiwyg plugins - needed patches too so they could be compatible with the Wysiwyg hook not being called on every page load (once the Wysiwyg patch has been applied).
Comment #9
chris matthews commentedClosing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team