Opened 3 years ago

Closed 3 years ago

#16838 closed Bug (wontfix)

Base path automatic detection fail

Reported by: Denis Owned by:
Priority: Normal Milestone:
Component: General Version: 4.0
Keywords: Cc:

Description

The code to set CKEditor base path automatically, for loading resources (config, skins, styles, translations, etc.), does not work on some specific situations.

Steps to Reproduce

File name and hierarchy matters to reproduce! Rename ckeditor.js with a "hash" prepend and set your html page and javascript resources in two different directories, like described below:

project/
|_ html/
   |_ page.html
|_ javascript/
    |_ 123abc-ckeditor.js
    |_ [other ckeditor files: config.js, styles.js, ...]

Content of page.html is as follow.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <title>basePath  bug</title>
  <script src="../javascript/123abc-ckeditor.js"></script>
</head>
<body>
  <textarea id="editor1"></textarea>
  <script>
    CKEDITOR.replace('editor1');
  </script>
</body>
</html>

Note: this has been tested with CKEditor Nighty (1 February 2017) on IE 11, Chrome 56, Firefox 51, Safari 10 and Opera 42. I did not test old IE and Edge.

Actual Result

CKEditor will try to load its resources (config.js, editor.css and en.js) from project/html/. Although, resources should be loaded from project/javascript/.

Expected result

I can accept this bad detection, as the filename is unusual and base path can be set manually. But, I am expecting the code to fail loudly: throw an exception telling the path could not be automatically detected. Has code seems to be design for...

Bug Reason

Bug is caused by code from basePath function in ckeditor_base.js, see https://github.com/ckeditor/ckeditor-dev/blob/2db0ce3e25f9c97404e2be107e1250a78254ddd7/core/ckeditor_base.js#L118-L150.

As 123abc-ckeditor.js does not match basePathSrcPattern:

/(^|.*[\\\/])ckeditor\.js(?:\?.*|;.*)?$/i;

The first if will let the path variable equal to the empty string.

The second if (who is 'IE only') will then be executed. Setting wrongly the path on actual location.

Finally the last if will never be executed whatever and is actually some dead code!

Proposed Solution

I propose to make a pull request changing the line:

if ( path.indexOf( ':/' ) == -1 && path.slice( 0, 2 ) != '//' ) {

by

if ( path && path.indexOf( ':/' ) == -1 && path.slice( 0, 2 ) != '//' ) {

This make the code throw as expected. On all tested browsers.

Change History (2)

comment:1 Changed 3 years ago by Tade0

Status: newconfirmed
Version: 4.7.0 (GitHub - major)4.0

Bug is confirmed to appear since at least 4.0, but the offending line appeared in 4.4.4.

comment:2 Changed 3 years ago by Marek Lewandowski

Resolution: wontfix
Status: confirmedclosed

@denisname Thank you for detailed, quality report, however this issue should not be confirmed from the very beginning.

The reason for it is that you're changing ckeditor.js file which is not something that you should do. The package should be simply placed as it is. If it collides with your other files (and in general) you should put it simply into a subdirectory, say project/javascript/ckeditor/ and leave files unchanged.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy