Skin sprite image caching causes garbled toolbar icons after upgrade

[Jeremy Jannotta]

We just upgraded from v4.1.1 -> v4.1.3, and upon viewing the first time, everyone sees garbled/unreadable toolbar icons. Refreshing or clearing cache fixes it, but it appears very broken the first time people load it after upgrade.

Looks like there's no cache version on the skin's icons.png file (ckeditor/skins/moono/icons.png).

We'd like to see the same cache timestamp that's applied to other files, applied to the sprite image, to prevent this going forward. So in the case of 4.1.3, the sprite url would somethinglook like 'ckeditor/skins/moono/icons.png?t=D6IC'.

[Jeremy Jannotta]

I will add that this issue would also apply to any other UI images loaded by the ckeditor, like for plugins, skins, dialogs, etc.

[Piotrek Koszuliński]

From what I see these files are loaded without a timestamp:

• icons strip,
• contents.css,
• close.png image used in a dialog.

Or other files - lang, css, dialogs etc are loaded with timestamps. At least icons strip and contents.css could be loaded with a timestamp too.

[Jakub Ś]

This ticket is related to #11625.

[Jakub Ś]

#11345 was marked as duplicate.

[bmulholland]

+1 - this is affecting my users on most ckeditor upgrades.

[Wiktor Walc]

Let's see what we can do with this.

[Matti Järvinen]

If those files are included in CSS -files there really isn't much that can be done.

Options in that case would be:

1. Add version number to every changed url() resource.
2. Load theme CSS by Ajax replace url().

[Piotrek Koszuliński]

The 4.4.3 milestone was shortened, so the remaining tickets must be postponed.

[Artur Delura]

Now when generating release version on CKEditor in CKBuilder we add random 6 digit hash to each image in CSS file. Changes are in ​https://github.com/ckeditor/ckbuilder/tree/t/10685.

[Piotrek Koszuliński]

Without changes in online builder this feature does not make sense yet, so until we've got all parts done we'll not be reviewing this.

[Piotrek Koszuliński]

Unfortunately, despite the importance of this issue, we will not have any resources to work on the builder part for at least next 2-3 months. This forces us to postpone this ticket even further, so currently the most reasonable milestone is 4.6.0.

[Jonathan]

Can we bump up the priority on this one? We inevitably have users that run into this issue when we update the editor to a new version.

[Piotrek Koszuliński]

4.6 is the closest possible milestone. 4.5 will be released in couple of days and change like this one cannot be included in a minor release. The good news is that 4.6 will not take so much time as 4.5. We we'll be back to our ~4 month release schedule.

In the meantime - we really do recommend serving every CKE version from a different directory (e.g. ckeditor/4.4.8/ckeditor.js). Unless you try to concatenate all files together this solves the issue.

[bmulholland]

It's worth mentioning that concatenating all files is considered best practices.

[Piotrek Koszuliński]

It's worth mentioning that concatenating all files is considered best practices.

Yes and no. If you have a website where CKEditor is used in 3 of 10 subpages, then it does not make sense to serve the same asset on all of these 10 subpages. It's better to split the asset so CKEditor can be loaded conditionally. Also, I cannot find the article now, but I remember reading that the optimal number of assets isn't really 1. If I remember correctly it was because browser waits so long to download it, while it could start parsing and executing some other file if the asset was split.

Anyway, not important here. We understand that some developers want to concatenate CKEditor with other scripts or that URL cannot be changed.

[Steve James]

+1 for this from me, also. This has been a source of pain for me with regards to CKEditor upgrades.

In the meantime - we really do recommend serving every CKE version from 
a different directory (e.g. ckeditor/4.4.8/ckeditor.js). Unless you try 
to concatenate all files together this solves the issue. 

I do not entirely agree with this approach for CKEditor. I recognize this is considered a best practice regarding cache management by many, but my view is that it really is most applicable to resources in the folder structure that do not end up getting referenced via <a>/<img> tags in the content.

A specific example would be the smileys. A huge headache that I ran into was that I was ending up with a huge bunch of broken <img tags for smileys when upgrading from one ckeditor version to another and using the folder version approach. (300,000+ users... major pain) The reason, of course, was that the smileys are essentially content that is bundled with the smileys plugin in editor builds, and they are inserted into the content via <img tags that reference their file folders in the current editor folder structure, *unless you specifically tell the editor to look in another folder location in the config*. It took me a while to sort out isolating the smileys folder separate from the cke folder structure and setting the smiley folder location in the editor config.

Therefore, the folder version approach can cause problems for anything that the editor *inserts into the content* via a link or image tag that references bundled content in the current folder structure. Since I do not know what types of content (beyond smileys) you may bundle with the editor in the future, I specifically choose not to use the folder version approach, and I am also very much in favor of using a version/timestamp query parameter instead. What I would prefer is that you include it as an version/timestamp value as "buried" default config setting that I can override via a config setting I specify--that will help with special cases where I may need to force a resource reload for my users. (A specific example is forcing a reload for mono vs color toolbar icons.)

[Jakub Ś]

#13897 was marked as duplicate.

[jkallay]

The priority of doing something about this should be elevated, because in the current state this (like all caching-related bugs) breaks things in an unpredictable way, and when it does break it happens in a spectacular way (in the case of a cached icons strip).

The big problem here is that CKEditor advertises that it takes care of cache busting with the build version it appends everywhere. To make matters worse, with the icons the markup makes it appear that the right thing is happening, because each toolbar button has an inline style with a resolved background image url and a build number--the version of the url without the build number is sneakily winning because the rule in the skin's editor.css is !important. If something is known to be broken, it's better for it to be broken in an obvious way.

[Marek Lewandowski]

This issue has been a subject of ​#2702171 in Drupal bug tracker.

[Tade0]

Created a pull request in the CKBuilder repo that solves this issue: ​https://github.com/ckeditor/ckbuilder/pull/12.

[Marek Lewandowski]

Looking good. All we're missing is putting 2.3.1 version at our host, and updating build.sh in ckeditor-dev and presets.

Let's try to ship it with 4.5.9.

[Tade0]

Fixed with commit d2155771c06a5a7437f30be9f3b42ef60bc184c2 in ​https://github.com/ckeditor/ckbuilder.

[Piotrek Koszuliński]

Hurray! Thanks guys :)

[bmulholland]

Yay! I've been watching this for years and I'm totally stoked to have it fixed. Thanks all!

[Marek Lewandowski]

Perfect, glad to hear it will help you! :)