IE6 skin images 404

[Niko Viitala]

IE6 (SP2) tries to load skin images two times. In the other time, it tries to search images in wrong path "/js/ckeditor/skins/kama/​http://URL/js/ckeditor/skins/kama/icons.png" etc, so those images are redirect to 404. The same seems to happen in skin v2 too. IE > 6 and other browsers doesn't have the bug.

Error log:

"GET /js/ckeditor/config.js?t=A7HG4HT HTTP/1.1" 200 1289
"GET /js/ckeditor/skins/kama/images/sprites_ie6.png?t=A7HG4HT HTTP/1.1" 304
"GET /js/ckeditor/skins/kama/icons.png?t=A7HG4HT HTTP/1.1" 304 304
"GET /js/ckeditor/skins/kama/images/dialog_sides.gif?t=A7HG4HT HTTP/1.1" 304
"GET /js/ckeditor/skins/kama/https://www.xxx.fi/js/ckeditor/skins/kama/icons.png?t=A7HG4HT&t=A7HG4HT HTTP/1.1" 404
"GET /js/ckeditor/skins/kama/https://www.xxx.fi/js/ckeditor/skins/kama/images/sprites_ie6.png?t=A7HG4HT&t=A7HG4HT HTTP/1.1" 404
"GET /js/ckeditor/skins/kama/https://www.xxx.fi/js/ckeditor/skins/kama/images/dialog_sides.gif?t=A7HG4HT&t=A7HG4HT HTTP/1.1" 404
"GET /js/ckeditor/skins/kama/https://www.xxx.fi/js/ckeditor/skins/kama/https://www.tyokyvyntuki.fi/js/ckeditor/skins/kama/icons.png?t=A7HG4HT&t=A7HG4HT&t=A7HG4HT HTTP/1.1" 404

Suggested fix: I deleted these lines (in compressed ckeditor.js or in skin.js) and it's working fine again. Maybe a better approach would be to fix preload function?:

        if ( CKEDITOR.env.ie && CKEDITOR.env.version < 7 )
        {
                // For IE6, we need to preload some images, otherwhise they will be
                // downloaded several times (CSS background bug).
                preload.push( 'icons.png', 'images/sprites_ie6.png', 'images/dialog_sides.gif' );
        }

[Alfonso Martínez de Lizarrondo]

I don't see a 404, but two images are loaded twice: one with the timestamp and the second one without it.

http://nightly.ckeditor.com/5829/_samples/replacebyclass.html
http://nightly.ckeditor.com/5829/ckeditor.js
http://nightly.ckeditor.com/5829/_samples/sample.js
http://nightly.ckeditor.com/5829/config.js?t=A7M74HW
http://nightly.ckeditor.com/5829/skins/kama/icons.png?t=A7M74HW
http://nightly.ckeditor.com/5829/skins/kama/images/sprites_ie6.png?t=A7M74HW
http://nightly.ckeditor.com/5829/skins/kama/images/dialog_sides.gif?t=A7M74HW
http://nightly.ckeditor.com/5829/skins/kama/editor.css?t=A7M74HW
http://nightly.ckeditor.com/5829/lang/es.js?t=A7M74HW
http://nightly.ckeditor.com/5829/skins/kama/icons.png
http://nightly.ckeditor.com/5829/skins/kama/images/sprites_ie6.png
http://nightly.ckeditor.com/5829/contents.css
http://nightly.ckeditor.com/5829/plugins/styles/styles/default.js?t=A7M74HW&t=A7M74HW

So we should certainly look at the reason of this problem.
I guess that the ones without the timestamp are loaded due to the stylesheet references.
And maybe the patch to avoid zoom problems in IE can expand this issue to all IE versions.

[Frédéric Arène]

I have the same problem and maybe I can help in term of context.

I have a page with multiple CKEditor windows.

The 404 occurs only when the Setting of Temporary Internet Files is "Verify on Each Visit" (translated from French).

It's clear that the URL's construction is at fault. Something should be cleared before the url to request is sent.

The 404 are like that : ​http://myurl/ckeditor/skins/kama/'''targetedRessource?'''t=timestamp ​http://myurl/ckeditor/skins/kama/'''''http://myurl/ckeditor/skins/kama/'''targetedRessource?'''t=timestamp'''&''t=timestamp ​http://myurl/ckeditor/skins/kama/''http://myurl/ckeditor/skins/kama/'''''http://myurl/ckeditor/skins/kama/'''targetedRessource?'''t=timestamp'''&''t=timestamp''&t=timestamp

It seems to me that in constructing the url, each CKEditor instance uses a variable from the previous instance (or a global, shared CKEDITOR variable ?) without clearing it.

[Frédéric Arène]

Sorry for the formatting mixup, what I meant was :

myurl/ckeditor/skins/kama/targetedRessource?t=timestamp
myurl/ckeditor/skins/kama/myurl/ckeditor/skins/kama/targetedRessource?t=timestamp&t=timestamp
myurl/ckeditor/skins/kama/myurl/ckeditor/skins/kama/myurl/ckeditor/skins/kama/targetedRessource?t=timestamp&t=timestamp&t=timestamp

or roughly : URL (1st instance) URL/URL (2nd instance) URL/URL/URL (3rd instance...)

[Frédéric Arène]

skin.js line34:

		var appendSkinPath = function( fileNames )
		{
			for ( var n = 0 ; n < fileNames.length ; n++ )
			{
				fileNames[ n ] = CKEDITOR.getUrl( paths[ skinName ] + fileNames[ n ] );
			}
		};

It seems that each instance execute this method. Multiple executions causes filename[n] to grow with an added paths[skinName] each time.

[Frédéric Arène]

In core/skins.js line 58 (V3.4.0) :

Check if preloaded if not then

preload (uses appendSkinPath) preloaded = 1

end if

Maybe while an instance is preloading, another instance comes around and because the preloading isn't finished and preloaded still undefined...

I tested with preloaded = 1 before executing the preload (thus prohibiting another instance from executing preload/appendSkinPath) -> no more 404.

[Tobiasz Cudnik]

After investigating the issue i can tell following things:

• Preloader's lock is badly designed leaving time hole, which results in duplicate preloading of same images (and each preload prepends URL)
• Preloading doesn't resolve IE6 cache issue as we're still having dozens of 304 for icons.png etc (check attached httpd.log).
• Issue is randomly reproducible on source and always on the release using a page with multiple instances with same skin.
• Double request to icons.png (with timestamp from JS and without it from CSS) should be fixed using CKReleaser, which should update filename with current timestamp during release.

I'm attaching the patch which solves 404s, although it could result in using image before it will be preloaded, but as i've pointed out preloading doesn't really work now.

I will be preparing next patches to fix the issue, probably by extending CKEDITOR.imageCacher with event system allowing multiple sources to bind to an preload-complete event.

[Tobiasz Cudnik]

This patch should keep preloading tight, without any time holes. I've also fixed 304s by enabling IE6 image background caching (which was commented out in our source).

There is #6347 for double request to image files because of timestamp.

[Garry Yao]

The patch is good, but if "document.execCommand( 'BackgroundImageCache', false, true )" resolves the ie6 image request problem, why should we still keep skin preload?

[Tobiasz Cudnik]

Replying to garry.yao:

The patch is good, but if "document.execCommand( 'BackgroundImageCache', false, true )" resolves the ie6 image request problem, why should we still keep skin preload?

Well, it's a good question. We don't need it anymore for IE6. Although i don't think we should loose this feature completely, as it might be useful in the future.

[Garry Yao]

As a decision from a group call, R+ for 6187_2.patch while we hold the removal of preloader with #6502.

[Garry Yao]

Fixed with [5982].