Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for lack of cache key for some resources #4852

Merged
merged 9 commits into from
Aug 27, 2021
6 changes: 3 additions & 3 deletions core/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ CKEDITOR.config = {
/**
* The CSS file(s) to be used to apply style to editor content. It should
* reflect the CSS used in the target pages where the content is to be
* displayed.
* displayed. Every URL is passed through {@link CKEDITOR#getUrl} function.
*
* **Note:** This configuration value is used only in {@glink guide/dev_framed `<iframe>`-based editor }
* and ignored by {@glink guide/dev_inline inline editor}
Expand All @@ -301,9 +301,9 @@ CKEDITOR.config = {
* config.contentsCss = '/css/mysitestyles.css';
* config.contentsCss = [ '/css/mysitestyles.css', '/css/anotherfile.css' ];
*
* @cfg {String/Array} [contentsCss=CKEDITOR.getUrl( 'contents.css' )]
* @cfg {String/Array} [contentsCss='contents.css']
*/
contentsCss: CKEDITOR.getUrl( 'contents.css' ),
contentsCss: 'contents.css',

/**
* Comma-separated list of plugins to be used in an editor instance. Note that
Expand Down
4 changes: 4 additions & 0 deletions core/dom/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ CKEDITOR.tools.extend( CKEDITOR.dom.document.prototype, {
*
* CKEDITOR.document.appendStyleSheet( '/mystyles.css' );
*
* **Note:** URLs are passed through {@link CKEDITOR#getUrl} function.
*
* @param {String} cssFileUrl The CSS file URL.
*/
appendStyleSheet: function( cssFileUrl ) {
cssFileUrl = CKEDITOR.getUrl( cssFileUrl );

if ( this.$.createStyleSheet )
this.$.createStyleSheet( cssFileUrl );
else {
Expand Down
7 changes: 5 additions & 2 deletions core/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@
* Builds a HTML snippet from a set of `<style>/<link>`.
*
* @param {String/Array} css Each of which are URLs (absolute) of a CSS file or
* a trunk of style text.
* a trunk of style text. URLs are passed through {@link CKEDITOR#getUrl} function.
* @returns {String}
*/
buildStyleHtml: function( css ) {
Expand All @@ -426,8 +426,11 @@
// Is CSS style text ?
if ( /@import|[{}]/.test( item ) )
retval.push( '<style>' + item + '</style>' );
else
else {
item = CKEDITOR.getUrl( item );

retval.push( '<link type="text/css" rel=stylesheet href="' + item + '">' );
}
}
}
return retval.join( '' );
Expand Down
157 changes: 157 additions & 0 deletions tests/core/ckeditor/geturl.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/* bender-tags: editor */

( function() {
'use strict';

var FAKE_BASE_PATH = 'http://some.example:1030/subdir/ckeditor/',
testCases = [
{
url: 'just.js',
expected: FAKE_BASE_PATH + 'just.js',
timestamp: ''
},

{
url: 'just.js',
expected: FAKE_BASE_PATH + 'just.js?t=347',
timestamp: '347'
},

{
url: './test.css',
expected: FAKE_BASE_PATH + './test.css',
timestamp: ''
},

{
url: './test.css',
expected: FAKE_BASE_PATH + './test.css?t=tr4',
timestamp: 'tr4'
},

{
url: '../whatever.js',
expected: FAKE_BASE_PATH + '../whatever.js',
timestamp: ''
},

{
url: '../whatever.js',
expected: FAKE_BASE_PATH + '../whatever.js?t=qwerty',
timestamp: 'qwerty'
},

{
url: '/file',
expected: '/file',
timestamp: ''
},

{
url: '/file',
expected: '/file?t=cke4',
timestamp: 'cke4'
},

{
url: 'http://some.site:47/file.js',
expected: 'http://some.site:47/file.js',
timestamp: ''
},

{
url: 'http://some.site:47/file.js',
expected: 'http://some.site:47/file.js?t=cv3',
timestamp: 'cv3'
},

{
url: 'https://whatever/file',
expected: 'https://whatever/file',
timestamp: ''
},

{
url: 'https://whatever/file',
expected: 'https://whatever/file?t=1er',
timestamp: '1er'
},

{
url: '//cksource.com/file.css',
expected: '//cksource.com/file.css',
timestamp: ''
},

{
url: '//cksource.com/file.css',
expected: '//cksource.com/file.css?t=try6',
timestamp: 'try6'
},

{
url: 'file.t?something=here',
expected: FAKE_BASE_PATH + 'file.t?something=here&t=wer',
timestamp: 'wer'
},

{
url: '/file.t?something=here',
expected: '/file.t?something=here&t=wer',
timestamp: 'wer'
},

{
url: 'http://dot.example/file.t?something=here',
expected: 'http://dot.example/file.t?something=here&t=wer',
timestamp: 'wer'
},

{
url: 'https://dot.example/file.t?something=here',
expected: 'https://dot.example/file.t?something=here&t=wer',
timestamp: 'wer'
},

{
url: '//dot.example/file.t?something=here',
expected: '//dot.example/file.t?something=here&t=wer',
timestamp: 'wer'
}
],
tests = createGetUrlTests( testCases );

bender.test( tests );

function createGetUrlTests( testCases ) {
return CKEDITOR.tools.array.reduce( testCases, function( tests, testCase ) {
var test = {},
testName = 'test CKEDITOR.getUrl( \'' + testCase.url + '\' ) with timestamp \'' +
testCase.timestamp + '\'';

test[ testName ] = createTest( testCase );

return CKEDITOR.tools.object.merge( tests, test );
}, {} );

function createTest( options ) {
var url = options.url,
expected = options.expected,
timestamp = options.timestamp;

return function() {
var originalBasePath = CKEDITOR.basePath,
originalTimestamp = CKEDITOR.timestamp,
actualResult;

CKEDITOR.basePath = FAKE_BASE_PATH;
CKEDITOR.timestamp = timestamp;
actualResult = CKEDITOR.getUrl( url );
CKEDITOR.basePath = originalBasePath;
CKEDITOR.timestamp = originalTimestamp;

assert.areSame( expected, actualResult, 'For ' + url + ' input correct ' + expected + ' is returned' );
Comandeer marked this conversation as resolved.
Show resolved Hide resolved
};
}
}
} )();
13 changes: 13 additions & 0 deletions tests/core/ckeditor/manual/geturl.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<div id="editor">
<p>Lorem ipsum dolor sit amet</p>
</div>

<script>
( function() {
if ( !CKEDITOR.timestamp ) {
return bender.ignore();
}
Comandeer marked this conversation as resolved.
Show resolved Hide resolved

CKEDITOR.replace( 'editor' );
} )();
</script>
10 changes: 10 additions & 0 deletions tests/core/ckeditor/manual/geturl.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
@bender-tags: bug, 4.17.0, 4761
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, undo, emoji, copyformatting, easyimage, tableselection

**Note**: This test requires CKEDITOR with set `CKEDITOR.timestamp` (e.g. built one).

1. Open developer tools and switch to "Network" tab.
2. Refresh the page.

**Expected** CKEditor 4's resources are loaded with the cache key (`?t=<some alphanum characters>` at the end of URL).
33 changes: 31 additions & 2 deletions tests/core/dom/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ bender.test( appendDomObjectTests(
assert.areSame( document, doc.$ );
},

test_appendStyleSheet: function() {
var cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css';
// (#4761)
test_appendStyleSheet_notimestamp: function() {
var originalTimestamp = CKEDITOR.timestamp,
cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css';

CKEDITOR.timestamp = '';

var doc = new CKEDITOR.dom.document( document );
doc.appendStyleSheet( cssUrl );
Expand All @@ -30,6 +34,31 @@ bender.test( appendDomObjectTests(
}
}

CKEDITOR.timestamp = originalTimestamp;
assert.isTrue( succeed, 'The link element was not found' );
},

// (#4761)
test_appendStyleSheet_timestamp: function() {
var originalTimestamp = CKEDITOR.timestamp,
cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css',
expectedUrl = cssUrl + '?t=wer56';

CKEDITOR.timestamp = 'wer56';

var doc = new CKEDITOR.dom.document( document );
doc.appendStyleSheet( cssUrl );

var links = document.getElementsByTagName( 'link' );
var succeed = false;
for ( var i = 0 ; i < links.length ; i++ ) {
if ( links[ i ].href == expectedUrl ) {
succeed = true;
break;
}
}

CKEDITOR.timestamp = originalTimestamp;
assert.isTrue( succeed, 'The link element was not found' );
},

Expand Down
74 changes: 74 additions & 0 deletions tests/core/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,80 @@
assert.areSame( 2, testSpy.callCount );
assert.areSame( testObj, testSpy.getCall( 0 ).thisValue );
assert.isTrue( testSpy.calledWithExactly( 'baz', 100, 'bar' ) );
},

// (#4761)
'test buildStyleHtml with no timestamp returns stylesheet URL without cache key for passed string': function() {
var originalTimestamp = CKEDITOR.timestamp,
url = '/file.css',
expectedHref = 'href="' + url + '"',
html;

CKEDITOR.timestamp = '';
html = CKEDITOR.tools.buildStyleHtml( url ),
CKEDITOR.timestamp = originalTimestamp;

assert.areNotSame( -1, html.indexOf( expectedHref ), 'Built HTML includes correct stylesheet link' );
},

// (#4761)
'test buildStyleHtml with set timestamp returns stylesheet URL with cache key for passed string': function() {
var originalTimestamp = CKEDITOR.timestamp,
newTimestamp = 'wer55',
url = '/file.css',
expectedHref = 'href="' + url + '?t=' + newTimestamp + '"',
html;

CKEDITOR.timestamp = newTimestamp;
html = CKEDITOR.tools.buildStyleHtml( url ),
CKEDITOR.timestamp = originalTimestamp;

assert.areNotSame( -1, html.indexOf( expectedHref ), 'Built HTML includes correct stylesheet link' );
},

// (#4761)
'test buildStyleHtml with no timestamp returns stylesheet URLs without cache key for passed array': function() {
var originalTimestamp = CKEDITOR.timestamp,
urls = [
'/file.css',
'/some-other-file'
],
expectedHrefs = [
'href="' + urls[ 0 ] + '"',
'href="' + urls[ 1 ] + '"'
],
html;

CKEDITOR.timestamp = '';
html = CKEDITOR.tools.buildStyleHtml( urls ),
CKEDITOR.timestamp = originalTimestamp;

CKEDITOR.tools.array.forEach( expectedHrefs, function( expectedHref ) {
assert.areNotSame( -1, html.indexOf( expectedHref ), 'Built HTML includes correct stylesheet link' );
} );
},

// (#4761)
'test buildStyleHtml with set timestamp returns stylesheet URLs with cache key for passed array': function() {
var originalTimestamp = CKEDITOR.timestamp,
newTimestamp = 'wer55',
urls = [
'/file.css',
'/some-other-file'
],
expectedHrefs = [
'href="' + urls[ 0 ] + '?t=' + newTimestamp + '"',
'href="' + urls[ 1 ] + '?t=' + newTimestamp + '"'
],
html;

CKEDITOR.timestamp = newTimestamp;
html = CKEDITOR.tools.buildStyleHtml( urls ),
CKEDITOR.timestamp = originalTimestamp;

CKEDITOR.tools.array.forEach( expectedHrefs, function( expectedHref ) {
assert.areNotSame( -1, html.indexOf( expectedHref ), 'Built HTML includes correct stylesheet link' );
} );
}
} );
} )();