Skip to content

Commit

Permalink
Merge pull request #4852 from ckeditor/t/4761
Browse files Browse the repository at this point in the history
Fix for lack of cache key for some resources
  • Loading branch information
sculpt0r committed Aug 27, 2021
2 parents 92173c6 + 62c9bc7 commit 70649e5
Show file tree
Hide file tree
Showing 9 changed files with 299 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ New Features:

Fixed Issues:

* [#4761](https://github.com/ckeditor/ckeditor4/issues/4761): Fixed: not all resources loaded by the editor respect the cache key.
* [#3757](https://github.com/ckeditor/ckeditor4/issues/3757): [Firefox] Fixed: images pasted from [clipboard](https://ckeditor.com/cke4/addon/clipboard) are not inserted as Base64-encoded images.
* [#4604](https://github.com/ckeditor/ckeditor4/issues/4604): Fixed: [`CKEDITOR.plugins.clipboard.dataTransfer#getTypes()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_plugins_clipboard_dataTransfer.html#method-getTypes) returns no types.
* [#4597](https://github.com/ckeditor/ckeditor4/issues/4597): Fixed: Incorrect color conversion for HSL/HSLA values in [`CKEDITOR.tools.color`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools_color.html).
* [#4596](https://github.com/ckeditor/ckeditor4/issues/4596): Fixed: Incorrect handling of HSL/HSLA values in [`CKEDITOR.tools.color`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools_color.html).

**API Changes:**

* [#4761](https://github.com/ckeditor/ckeditor4/issues/4761): [`CKEDITOR.dom.document#appendStyleSheet()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dom_document.html#method-appendStyleSheet) and [`CKEDITOR.tools.buildStyleHtml()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools.html#method-buildStyleHtml) now use [`CKEDITOR.getUrl()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR.html#method-getUrl) to correctly handle caching of CSS files.
* [#4604](https://github.com/ckeditor/ckeditor4/issues/4604): Added the [`CKEDITOR.plugins.clipboard.dataTransfer#isFileTransfer()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_plugins_clipboard_dataTransfer.html#method-isFileTransfer) method.
* [#4583](https://github.com/ckeditor/ckeditor4/issues/4583): Added support for new, comma-less color syntax to [`CKEDITOR.tools.color`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools_color.html).
* [#4467](https://github.com/ckeditor/ckeditor4/issues/4467): Inserting content next to block [widgets](https://ckeditor.com/cke4/addon/widget) using keyboard navigation is now possible. Thanks to [bunglegrind](https://github.com/bunglegrind)!
Expand Down
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, 'Invalid URL generated from the ' + url );
};
}
}
} )();
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 ( bender.tools.env.mobile || !CKEDITOR.timestamp ) {
return bender.ignore();
}

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' );
} );
}
} );
} )();

0 comments on commit 70649e5

Please sign in to comment.