On a past upgrade, I think it was 4.0.x (not 100% sure what version) to 4.3.1 the Editor has not reloaded the JS and images from CKEditor saved in sites/all/libraries. The editor was completely broken (all buttons) as something with the image sprites has changed.

Root cause is that Drupal .htaccess file send a 2 week cache expire header.

# Cache all files for 2 weeks after access (A).
ExpiresDefault A1209600

After the upgrade all CKEditors have been broken for about 2 weeks on client side. I fear this may happen again in future upgrades. I guess that CKEditor is not appending the css_js_query_string url parameter to all ckeditor files what breaks the re-use of the cache clear feature used in core.

Example code:

$query_string = '?' . variable_get('css_js_query_string', '0');

I mark the case as critical as CKEditor becomes unusable for many weeks - until the client cache expires or users clear their browser cache manually. The problem here is they do not understand this.

Is this fixable in the module/ckeditor code?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alcroito’s picture

It's a bug in the CKEditor module in these lines of codes ckeditor.lib.inc

  // add custom stylesheet if configured
  // lets hope it exists but we'll leave that to the site admin
  $query_string = '?' . substr(variable_get('css_js_query_string', '0'), 0, 1);

It shouldn't take only the first character of the cache buster CSS string, but rather the whole string.
Just replace that with.

  $query_string = '?' . variable_get('css_js_query_string', '0');

Also attaching a patch (allthough not rolled against DEV version).

  • jcisio committed b29372f on 7.x-1.x authored by Placinta
    Issue #2330515 by Placinta | hass: Fixed Cached files used after editor...
jcisio’s picture

I committed patch #1 because it is evident. But it won't fix the bigger problem, because all JS/CSS/images files in the CKEditor library don't use this cache buster. There are several issue about it and this one should be marked as duplicated.

I think the proper solution is a/ either introduce a cache buster in CKEditor library b/ use a universal solution such as prefixing the CKEditor library with a cache buster hash (but in this case we need to modify the Libraries module too).

hass’s picture

Thanks for feedback. I'm not so deep in ckeditor itself, but cannot believe the developers have no solution implemented for this. I hoped you may missed to implement something in the module that allows a cache clear... Maybe there is something we are not aware of yet?

hass’s picture

Just found something we may use. See http://stackoverflow.com/questions/14940452/force-ckeditor-to-refresh-co...

CKEDITOR.timestamp='ABCD';

And we can use drupal

  variable_get('css_js_query_string', '0');

into this value and we are done...

leymannx’s picture

Thanks #5 for that hint. I wrote a small custom module (Github Link) that sets CKEDITOR.timestamp = +new Date and by that forces only the freshest files to be loaded since it appends a unique timestamp query to my custom JS.

larskleiner’s picture

Status: Active » Reviewed & tested by the community

Thanks @leymannx, your module fixes my CKEditor caching issue.

I couldn't get changes to my custom CKEditor plugin appear consistently but now they do.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: ckeditor-css-cache-buster-string.patch, failed testing.

natuk’s picture

Same here: leymannx's module from #6 fixes the problem.

milos.kroulik’s picture

Please note that module mentioned in #6 likely has serious issue: https://github.com/leymannx/drupal-ckeditor-timestamp/issues/2 Can you confirm, that enabling that module blocks Drupal batch operations?

milos.kroulik’s picture

Status: Needs work » Needs review
leymannx’s picture

I can't confirm this https://github.com/leymannx/drupal-ckeditor-timestamp/issues/2 issue. Batch operations, or deleting users work just fine. Anyways, you are supposed to have this module only activated during development.

jwilson3’s picture

The fundamental error is that as web developers tweaking things, we cannot depend on the CKEditor.timestamp from the compiled version of the CKEditor library js file, because that value only changes when the library version is updated. Our configuration files change much more frequently than that, and should not depend on CKEditor's internal cache-busting mechanism, nor should developers need to hack CKEditor's timestamp functionality as proposed in #6, because Drupal has its own robust standard cache-busting system that we can use.

The CKEditor module is already using the standard Drupal css_js_query_string in a couple of places, but not everywhere, so the patch here just completes the work to ensure that:

1) ckeditor.config.js file is always cache-busted when Drupal css/js cache is cleared (instead of checking the file modification time on every page load).
2) ckeditor.styles.js file is always cache-busted when Drupal css/js cache is cleared (instead of depending on the CKEditor.timestamp from the compiled version of the Ckeditor library).

Note that the Drupal cache-busting mechanism used here (which adds ?XXXXX to the end of the filename on the server side via php) does not interfere with CKEditor's own cache-busting method which adds &t=XXXX to the end of the filename on client side (via js).

Reviews appreciated!

Thanks.

jwilson3’s picture

Title: Cached files used after editor update » Cache-bust CKEditor Styles and Config files

Status: Needs review » Needs work

The last submitted patch, 13: ckeditor-styles-config-cache-bust-2330515-13.patch, failed testing. View results

jwilson3’s picture

Status: Needs work » Needs review
FileSize
2.49 KB

Reroll to fix relative path so patch can apply.

llslim’s picture

I tried the patch in [#12266257-12238240], and it worked for reloading the editor with new config

dat deaf drupaler’s picture

I can confirm patch from #16 works, thank you!! Drove me insane trying to revise/test customising list of styles in ckeditor and flushing cache repeatedly to update the changes in drop-list.

Hats off to you, @jwilson3 !

trouble.tribbles’s picture

Patch #16 didn't have any effect for me but the module did.

TristanTheKnightAway’s picture

CKeditor Pro Tip:

If you have more than two classes assigned to elements in your ckeditor.styles.js file they need to be in alphabetical order to render.

This will work:
{ name: 'Alert Success', element: 'div', attributes: { 'class': 'alert alert--success' } },

This WILL NOT:
{ name: 'Alert Success', element: 'div', attributes: { 'class': 'alert--success alert' } },

because insanity.

ron_s’s picture

Might want to try this patch, which has already been committed for the WYSIWYG module, and essentially does the same thing for CKEditor: https://www.drupal.org/project/ckeditor/issues/2679179

It is also the approach used to fix the problem in Drupal 8: https://www.drupal.org/project/drupal/issues/2679903

ron_s’s picture

vokiel’s picture

Attaching a new patch, since the one from #16 couldn't be applied.

Also, attaching interdiff (after changes from #16 applied manually).

  • vokiel committed 3f01c1e on 7.x-1.x
    Issue #2330515 by jwilson3, vokiel, alcroito: Cache-bust CKEditor Styles...
vokiel’s picture

Status: Needs review » Fixed
vokiel’s picture

Status: Fixed » Closed (fixed)