Opened 7 years ago
Closed 7 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 7 years ago by
| Status: | new → confirmed |
|---|---|
| Version: | 4.7.0 (GitHub - major) → 4.0 |
comment:2 Changed 7 years ago by
| Resolution: | → wontfix |
|---|---|
| Status: | confirmed → closed |
@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.
Bug is confirmed to appear since at least 4.0, but the offending line appeared in 4.4.4.