Needs review
Project:
Wysiwyg
Version:
7.x-2.x-dev
Component:
Code
Priority:
Major
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
30 Jun 2010 at 16:20 UTC
Updated:
28 Feb 2017 at 18:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
smk-ka commentedNice but didn't work for me, since we still need to include the files when loading lists from cache in wysiwyg_load_all_includes(). Also added reset flags at appropriate places, which are triggered on the wysiwyg profiles admin pages to reset the cached editor and plugin lists. Patches for D7 and D6 attached.
Comment #2
catchStill applies, re-rolled d7 patch for fuzz/offset/windows line breaks.
Comment #3
sunComment #4
helmo commentedLooks good. And coder module has no complaints.
Comment #5
twodWon't this break plugins which add additional scripts in hook_wysiwyg_plugin(), like media module does?
For reference: #1039076: Allow plugins to specify libraries and multiple javascript files
Comment #6
helmo commentedI did a quick test with the media module. It's media browser icon still works fine with the patch.
wysiwyg_get_all_plugins already has some caching, this is more about the lower level of which include files to load.
Comment #7
sunThanks for reporting, reviewing, and testing! Committed to 7.x-2.x.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.
Comment #8
acrollet commentedSorry to report that #5 is correct, this change breaks the media wysiwyg plugin. (It works on the first page load after cache clear, but is broken for every page load thereafter, which is perhaps why helmo didn't see the problem) Would it be possible to add a 'cachable' parameter to the plugin definition, or alternatively a 'additional_js_callback' parameter?
Comment #9
sunComment #10
sunHrm. Do we need to roll this back until #1039076: Allow plugins to specify libraries and multiple javascript files has been fixed and postpone it until Media has been changed accordingly and has a new release?
Comment #11
twodI say revert and wait. No matter if we get #1039076 in or add another callback as suggested by @acrollet, we'll still have to wait for Media to catch up, and they may not even be aware of the changes yet. To be sure they are, I created #1795968: Prepare for wysiwyg_load_includes being cached.
I hope they will be able to review the patches I added to #1039076 and spot any issues I've overlooked. If they think it looks good, we can commit that patch and re-commit this patch, and preferably sync the next release of both modules.
I'll make a sweep of the modules I know have plugins and check if they need to be changed as well.
Comment #12
sunReverted the commit.
Comment #13
socialnicheguru commentedany update on this? I would like to cache all the js and css associated with the tiny_mce library that is loaded as part of the module
Comment #14
twod@SocialNicheGury The situation has not really changed since the other patch mentioned in #10 is not yet committed.
The next release will be regression fixes, after that I will look at implementing new features.
The patch here will not actually cache the CSS and JS needed by editor libraries, only the meta-information about which files are needed.
We've discovered not many editor libraries so far can handle being aggregated by Drupal. The problems usually come down to missing semi-colons, which cause syntax errors when removing newlines. There are sometimes other problems as well, such as libraries trying to find their base path by searching for the script tags which include them, and otherwise failing to load assets.
We do try to aggregate library files where possible but due to differences between library versions and the many per-version overrides that would lead to we usually have to default to not doing so. You could try implementing
hook_wysiwyg_editor_alter()to modify the editor definition and in there the options it passes along todrupal_add_js()if you wish to experiment.Comment #15
socialnicheguru commented@TwoD - thanks for the explanation. I am profiling my install now. I notice that the tinymce wyswig libraries are taking forever to load.
Comment #16
twodDo you mean the integration files under wysiwyg/editors/js?
If so, those should be aggregated with the rest of Drupals scripts when needed. Above I was only talking about the actual library files in sites/all/libraries/tinymce/... Those can often not be aggregated, but like I said you can try for your current version of TinyMCE by implementing
hook_wysiwyg_editor_alter()and changing the library files options array.You'll likely need to change it somewhere at
$editors['tinymce']['versions']['4']['libraries']['min']['files'];, if you're using TinyMCE 4. It's currently just an array with a string, but that array basically allows for all options you can send todrupal_add_js()by changing the current string value to be a key in an associative array where the value is an array passed along todrupal_add_js, see$editors['tinymce']['versions']['3.3']['libraries']['']['files']['tiny_mce.js'];.