Opened 14 years ago
Closed 14 years ago
#4767 closed Bug (fixed)
CKEditor does not work when ckeditor_source.js is loaded in the <body> tag
| Reported by: | Wiktor Walc | Owned by: | Alfonso Martínez de Lizarrondo |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.2 |
| Component: | General | Version: | SVN (CKEditor) - OLD |
| Keywords: | Confirmed Review+ | Cc: |
Description
When
<script type="text/javascript" src="ckeditor_source.js"></script>
is placed inside of the <body> tag, CKEditor doesn't load (tested on FF 3.5.5, IE8, Opera 10.. probably doesn't work in every browser).
Error in Firefox:
CKEDITOR.env is undefined http://example.com/ckeditor_31/_source/core/loader.js Line 165
Attachments (7)
Change History (22)
comment:1 Changed 14 years ago by
| Milestone: | → CKEditor 3.2 |
|---|
comment:2 Changed 14 years ago by
| Summary: | CKEditor does not work when ckeditor_cource.js is loaded in the <body> tag → CKEditor does not work when ckeditor_source.js is loaded in the <body> tag |
|---|
comment:3 Changed 14 years ago by
| Keywords: | Review? added |
|---|
comment:4 Changed 14 years ago by
| Keywords: | Review- added; Review? removed |
|---|
The patch works well for this case, while it brings a significant drawback, by simply force 'document.write' to load script will bring regression when 'ckeditor_source.js' is created dynamically after page has loaded (aka. lazy load), the things should be fixed are instead:
- The 'document.body' check;
- The incorrect 'CKEDITOR.env' reliance.
Changed 14 years ago by
| Attachment: | loadscript.zip added |
|---|
This is the demo for different thing between creating a script by using 'document.write' and creating dynamic script tag
comment:5 Changed 14 years ago by
| Keywords: | Review? added; Review- removed |
|---|---|
| Owner: | set to Minh Nguyen |
| Status: | new → assigned |
In this case we use 'document.write' to keep the browser having one process to render. When we use other ways, some browsers open a new process to load script. Then we'll have error if access to variables in the exented file has not yet been loaded.
comment:6 Changed 14 years ago by
We won't go for loadPending method because it's useless in this case. We need to continue using 'document.write' as we have used in line 23 of ckeditor_source.js before.
comment:7 Changed 14 years ago by
| Keywords: | Review- added; Review? removed |
|---|
document.write CANNOT be used as the exclusive loading method here. This brings issues, as previously pointed by Garry.
To have a better understanding, please check our docs to understand all the possible loading methods we offer with CKEditor. The document.write approach will work with the "Inline" method only. It'll instead break the page when using the "Deferred", "On Demand" or "Timeout" methods.
To test it, its enough to replace "ckeditor_source.js" with "ckeditor_basic_source.js" in the trunk samples. After the patch, things will get broken.
comment:8 Changed 14 years ago by
| Owner: | changed from Minh Nguyen to new |
|---|---|
| Status: | assigned → new |
Changed 14 years ago by
| Attachment: | load samples.zip added |
|---|
samples of different ways to load CKEditor
comment:9 Changed 14 years ago by
| Keywords: | Review? added; Review- removed |
|---|---|
| Owner: | changed from new to Alfonso Martínez de Lizarrondo |
| Status: | new → assigned |
The zip includes 5 different ways of loading the editor with the scripts in the body, the fifth one is the one that requires more changes, as a document.write was used in ckeditor_source and ckeditor_basic_source to load _source/core/loader.js
The check for document.body was done in order to check if the page is fully loaded, in that situation we can't use document.write because it will blank the current document. We can change it checking also for document.readyState=='complete'. In loader.js this means that if it's possible it will try to add the scripts inline, without using timeouts or asynchronous loading. In the ckeditor_source and ckeditor_basic_source it's a must to avoid the document.write
The problem with document.readyState is that it isn't supported in Firefox < 3.6, so this feature won't be available for those versions.
Finally the core/ckeditor_basic.js is there to trigger the call to the function if the window has already been loaded by the time that file is loaded.
Maybe the changes in ckeditor_source and ckeditor_basic_source can be simplified to always use body.appendChild instead of document.write (bringing us one step closer to make CKEditor work in contentType="application/xhtml+xml" #4576) but that should be tested carefully to avoid breaking anything that currently works.
comment:10 Changed 14 years ago by
| Keywords: | Review- added; Review? removed |
|---|
Great sample pages here to illustrate! while I'm against the 5th one, in which you were even lazy loading the basic codes. According to our current design, I have to say you're adding a new feature with this sample here, since the basic codes (ckeditor_basic.js) is supposed to be always presented in host document, there're two reasons behind this:
- It was small enough to be excluded from considering lazy load;
- It contains bootstrap APIs like CKEDITOR.replace, which was supposed to immediately available in place (which makes dynamic script load infeasible).
To fix the problem described at the ticket here without targeting the lazy load function states above, your changes to loader.js in the patch is already enough.
comment:11 Changed 14 years ago by
| Keywords: | Review? added; Review- removed |
|---|
I've left in the changes to ckeditor_basic.js as they can be a little protection against situations where the page has finished loading by the time that the file is executed, they might not be easy to reproduce, but it should be a safe check: if the document is finished, then don't try to wait for the onload event because it will never fire.
I'll try to create testcases from the load samples 1-4 as soon as I finish to setup the testing environment.
comment:12 Changed 14 years ago by
as they can be a little protection against situations where the page has finished loading by the time that the file is executed...
I'm OK if you can provide an example, for me that's barely possible.
comment:14 Changed 14 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
The problem is that we should use document.write in that case, but the loader is using document.body to check whether to use it or not.