diff --git a/CHANGES.md b/CHANGES.md index 85fb8d3ac0e..0fdcffaa6a0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,10 +12,13 @@ Fixed issues: * [#4875](https://github.com/ckeditor/ckeditor4/issues/4875): Fixed: Not possible to delete multiple selected lists. * [#4873](https://github.com/ckeditor/ckeditor4/issues/4873): Fixed: Pasting content from MS Word and Outlook with horizontal lines prevents images from being uploaded. * [#4952](https://github.com/ckeditor/ckeditor4/issues/4952): Fixed: Dragging and dropping images within a table cells appends additional elements. +* [#4761](https://github.com/ckeditor/ckeditor4/issues/4761): Fixed: not all resources loaded by the editor respect the cache key. API changes: * [#4918](https://github.com/ckeditor/ckeditor4/issues/4918): Explicitly set [`config.useComputedState`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-useComputedState) default value to `true`. Thanks to [Shabab Karim](https://github.com/shabab477)! +* [#4761](https://github.com/ckeditor/ckeditor4/issues/4761): [`CKEDITOR.appendTimestamp()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR.html#method-appendTimestamp) function was added. +* [#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 newly added [`CKEDITOR.appendTimestamp()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR.html#method-appendTimestamp) function to correctly handle caching of CSS files. Other changes: diff --git a/ckeditor.js b/ckeditor.js index 203ce84ed46..a6129132c03 100644 --- a/ckeditor.js +++ b/ckeditor.js @@ -7,7 +7,7 @@ /* jshint ignore:start */ /* jscs:disable */ // replace_start -window.CKEDITOR||(window.CKEDITOR=function(){var o,d=/(^|.*[\\\/])ckeditor\.js(?:\?.*|;.*)?$/i,e={timestamp:"",version:"%VERSION%",revision:"%REV%",rnd:Math.floor(900*Math.random())+100,_:{pending:[],basePathSrcPattern:d},status:"unloaded",basePath:function(){var t=window.CKEDITOR_BASEPATH||"";if(!t)for(var e=document.getElementsByTagName("script"),n=0;n= 0 ? '&' : '?' ) + 't=' + this.timestamp; + resource = this.appendTimestamp( resource ); return resource; }, + /** + * Appends {@link CKEDITOR#timestamp} to the provided URL as querystring parameter ("t"). + * + * Leaves the URL unchanged if it is a directory URL or it already contains querystring parameter. + * + * @since 4.17.2 + * @param {String} resource The resource URL to which the timestamp should be appended. + * @returns {String} The resource URL with cache key appended whenever possible. + */ + appendTimestamp: function( resource ) { + if ( !this.timestamp || + resource.charAt( resource.length - 1 ) === '/' || + ( /[&?]t=/ ).test( resource ) + ) { + return resource; + } + + var concatenateSign = resource.indexOf( '?' ) >= 0 ? '&' : '?'; + return resource + concatenateSign + 't=' + this.timestamp; + }, + /** * Specify a function to execute when the DOM is fully loaded. * diff --git a/core/dom/document.js b/core/dom/document.js index 438b1fc8738..2ada517a963 100644 --- a/core/dom/document.js +++ b/core/dom/document.js @@ -43,6 +43,8 @@ CKEDITOR.tools.extend( CKEDITOR.dom.document.prototype, { * @param {String} cssFileUrl The CSS file URL. */ appendStyleSheet: function( cssFileUrl ) { + cssFileUrl = CKEDITOR.appendTimestamp( cssFileUrl ); + if ( this.$.createStyleSheet ) this.$.createStyleSheet( cssFileUrl ); else { diff --git a/core/tools.js b/core/tools.js index 4083a319f70..201cdb11f2a 100644 --- a/core/tools.js +++ b/core/tools.js @@ -424,10 +424,12 @@ for ( var i = 0; i < css.length; i++ ) { if ( ( item = css[ i ] ) ) { // Is CSS style text ? - if ( /@import|[{}]/.test( item ) ) + if ( /@import|[{}]/.test( item ) ) { retval.push( '' ); - else + } else { + item = CKEDITOR.appendTimestamp( item ); retval.push( '' ); + } } } return retval.join( '' ); diff --git a/tests/core/ckeditor/appendtimestamp.js b/tests/core/ckeditor/appendtimestamp.js new file mode 100644 index 00000000000..628c0f52013 --- /dev/null +++ b/tests/core/ckeditor/appendtimestamp.js @@ -0,0 +1,61 @@ +/* bender-tags: editor */ + +( function() { + 'use strict'; + + bender.test( { + 'test append timestamp returns the same resource path if there is no CKEDITOR.timestamp': performTimestampTest( + '', + 'style.css', + 'style.css', + 'Resource path was modified without timestamp in CKEDITOR' + ), + + 'test append timestamp returns resource path with timestamp if there is CKEDITOR.timestamp': performTimestampTest( + 'cke4', + 'style.css', + 'style.css?t=cke4', + 'Resource path was modified without timestamp in CKEDITOR' + ), + + 'test append timestamp returns the same resource path if it is a directory path': performTimestampTest( + 'cke4', + 'style/', + 'style/', + 'Resource path to directory should not be changed' + ), + + 'test append timestamp to resource path, with timestamp as single URL param, does not change it': performTimestampTest( + 'cke4', + 'style.css?t=qaz1', + 'style.css?t=qaz1', + 'Resource path with timestamp was affected with new timestamp' + ), + + 'test append timestamp to resource path with chained timestamp does not change it': performTimestampTest( + 'cke4', + 'style.css?q=abc&t=qaz1', + 'style.css?q=abc&t=qaz1', + 'Resource path with chained timestamp was affected with new timestamp' + ), + + 'test append timestamp to resource path with URL params adds timestamp as another param': performTimestampTest( + 'cke4', + 'style.css?q=abc', + 'style.css?q=abc&t=cke4', + 'Resource path with single param has not been affected by timestamp' + ) + } ); + + function performTimestampTest( timestamp, resourcePath, expected, message ) { + return function() { + var originalTimestamp = CKEDITOR.timestamp; + CKEDITOR.timestamp = timestamp; + + var result = CKEDITOR.appendTimestamp( resourcePath ); + + CKEDITOR.timestamp = originalTimestamp; + assert.areSame( expected, result, message ); + }; + } +} )(); diff --git a/tests/core/ckeditor/geturl.js b/tests/core/ckeditor/geturl.js new file mode 100644 index 00000000000..e1c3cc0bb46 --- /dev/null +++ b/tests/core/ckeditor/geturl.js @@ -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 ); + }; + } + } +} )(); diff --git a/tests/core/ckeditor/manual/assetscachekey.html b/tests/core/ckeditor/manual/assetscachekey.html new file mode 100644 index 00000000000..9f5e949023f --- /dev/null +++ b/tests/core/ckeditor/manual/assetscachekey.html @@ -0,0 +1,13 @@ +
+

Lorem ipsum dolor sit amet

+
+ + diff --git a/tests/core/ckeditor/manual/assetscachekey.md b/tests/core/ckeditor/manual/assetscachekey.md new file mode 100644 index 00000000000..e201fe90e18 --- /dev/null +++ b/tests/core/ckeditor/manual/assetscachekey.md @@ -0,0 +1,10 @@ +@bender-tags: bug, 4.17.2, 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=` at the end of URL). diff --git a/tests/core/dom/document.js b/tests/core/dom/document.js index 7a164ba7ea9..bfac3ef6239 100644 --- a/tests/core/dom/document.js +++ b/tests/core/dom/document.js @@ -15,21 +15,41 @@ bender.test( appendDomObjectTests( assert.areSame( document, doc.$ ); }, - test_appendStyleSheet: function() { - var cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css'; + // (#4761) + 'test appendStyleSheet without timestamp': function() { + var originalTimestamp = CKEDITOR.timestamp, + cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css'; + CKEDITOR.timestamp = ''; 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 == cssUrl ) { - succeed = true; - break; - } - } + doc.appendStyleSheet( cssUrl );// this makes absolute URL no matter what is passed + + var links = document.getElementsByTagName( 'link' ), + succeed = CKEDITOR.tools.array.some( links, function( link ) { + return link.href === cssUrl; + } ); + + CKEDITOR.timestamp = originalTimestamp; + assert.isTrue( succeed, 'The link element was not found' ); + }, + + // (#4761) + 'test appendStyleSheet with timestamp': function() { + var originalTimestamp = CKEDITOR.timestamp, + fakeTimestamp = 'cke4', + cssUrl = CKEDITOR.basePath + 'tests/_assets/sample.css', + expectedCssUrl = cssUrl + '?t=' + fakeTimestamp; + + CKEDITOR.timestamp = fakeTimestamp; + var doc = new CKEDITOR.dom.document( document ); + doc.appendStyleSheet( cssUrl );// this makes absolute URL no matter what is passed + + var links = document.getElementsByTagName( 'link' ), + succeed = CKEDITOR.tools.array.some( links, function( link ) { + return link.href === expectedCssUrl; + } ); + CKEDITOR.timestamp = originalTimestamp; assert.isTrue( succeed, 'The link element was not found' ); }, diff --git a/tests/core/tools.js b/tests/core/tools.js index 0aa0a86c302..8d3d508260e 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -1177,6 +1177,85 @@ assert.areSame( 2, testSpy.callCount ); assert.areSame( testObj, testSpy.getCall( 0 ).thisValue ); assert.isTrue( testSpy.calledWithExactly( 'baz', 100, 'bar' ) ); + }, + + // (#4761) + 'test buildStyleHtml returns relative URL for passed relative URL string': function() { + var relativeUrl = '/file.css', + styledStringElem = CKEDITOR.tools.buildStyleHtml( relativeUrl ); + + assert.areSame( -1, styledStringElem.indexOf( 'http' ), 'http should not be present in relative URL' ); + }, + + // (#4761) + 'test buildStyleHtml returns absolute URL for passed absolute URL string': function() { + var relatedUrl = 'http://example.com/file.css', + styledStringElem = CKEDITOR.tools.buildStyleHtml( relatedUrl ), + httpPosition = styledStringElem.indexOf( 'http' ); + + assert.isTrue( httpPosition > -1 , 'Absolute URL missed http protocol' ); + }, + + // (#4761) + 'test buildStyleHtml returns passed style text embedded in style element': function() { + var styleText = '*{color:red}', + expected = '', + styledStringElem = CKEDITOR.tools.buildStyleHtml( styleText ); + + assert.areSame( expected, styledStringElem, 'Styled text was not exact same wrapped in style element' ); + }, + + // (#4761) + 'test buildStyleHtml with no timestamp returns stylesheet URL without cache key for passed string': function() { + var originalTimestamp = CKEDITOR.timestamp, + relativeUrl = '/file.css', + expectedHref = 'href="' + relativeUrl + '"', + html; + + CKEDITOR.timestamp = ''; + html = CKEDITOR.tools.buildStyleHtml( relativeUrl ); + var expectedPosition = html.indexOf( expectedHref ); + + CKEDITOR.timestamp = originalTimestamp; + assert.isTrue( expectedPosition > -1, 'Built HTML does not contains expected href attribute' ); + }, + + // (#4761) + 'test buildStyleHtml adds timestamp as cache key to provided URL': function() { + var originalTimestamp = CKEDITOR.timestamp, + relativeUrl = '/file.css', + fakeTimestamp = 'cke4', + expectedHref = 'href="' + relativeUrl + '?t=' + fakeTimestamp + '"', + html; + + CKEDITOR.timestamp = fakeTimestamp; + html = CKEDITOR.tools.buildStyleHtml( relativeUrl ); + var expectedPosition = html.indexOf( expectedHref ); + + CKEDITOR.timestamp = originalTimestamp; + assert.isTrue( expectedPosition > -1, 'Built HTML does not contains expected href with timestamp' ); + }, + + // (#4761) + 'test buildStyleHtml adds timestamp as cache key to provided URLs': function() { + var originalTimestamp = CKEDITOR.timestamp, + relativeUrls = [ '/file.css', '../file2.css' ], + fakeTimestamp = 'cke4', + expectedHrefs = [ + 'href="' + relativeUrls[ 0 ] + '?t=' + fakeTimestamp + '"', + 'href="' + relativeUrls[ 1 ] + '?t=' + fakeTimestamp + '"' + ], + html; + + CKEDITOR.timestamp = fakeTimestamp; + html = CKEDITOR.tools.buildStyleHtml( relativeUrls ); + + CKEDITOR.timestamp = originalTimestamp; + + CKEDITOR.tools.array.forEach( expectedHrefs, function( expectedHref ) { + var expectedPosition = html.indexOf( expectedHref ); + assert.isTrue( expectedPosition > -1, 'Built HTML does not contains expected hrefs with timestamp' ); + } ); } } ); } )();