Ticket #7153 (closed Bug: fixed)
Can't dynamically load ckeditor_source.js
| Reported by: | damo | Owned by: | garry.yao |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.6.2 |
| Component: | General | Version: | 3.0 |
| Keywords: | IBM | Cc: | satya, jamescun, tmonahan |
Description
ckeditor_source.js uses document.write() when loading, this causes clearing of window variables when called at the end of a document load.
Proposal is that ckeditor_source.js (and ckeditor_basic_source.js) does something like this instead:
var head = document.getElementsByTagNam('head')[0];
var script = document.createElement('script');
script.type = 'text/javascript';
script.src = CKEDITOR.getUrl('_source/core/loader.js');
head.appendChild(script);
Attachments
Change History
comment:2 Changed 2 years ago by fredck
- Keywords Haspatch? removed
- Owner set to fredck
- Status changed from new to review
comment:3 Changed 2 years ago by garry.yao
- Status changed from review to review_failed
- Cc changed from satya,jamescun,tmonahan to satya, jamescun, tmonahan
The patch doesn't work for "interactive" document state which is quite a common case when using "DOMContentReady" event or padding body script block illustrated with the sample file.
7153.html is not working in at least FF4/IE8/IE9.
comment:4 Changed 2 years ago by fredck
- Status changed from review_failed to new
- Owner fredck deleted
@garry.yao, I have some doubts about the possibility of supporting your TC. I did some research and I didn't find a good solution that will fit all needs.
The patch makes it possible to load the editor *after* document load. This is a common use case as well.
@damo, you may tell us which kind of support we should have here. Maybe the patch already gives what you want. Let us know. Thanks.
comment:6 Changed 22 months ago by damo
Thanks Fred... this patch has been verified on our end and looks good.
comment:8 Changed 21 months ago by garry.yao
- Owner changed from fredck to garry.yao
@garry.yao, I have some doubts about the possibility of supporting your TC. I did some research and I didn't find a good solution that will fit all needs.
Can you check if the new patch is enough for resolving this case?
comment:9 Changed 21 months ago by garry.yao
Original ticket test: http://ckeditor.t/tt/7153/1.html
The extended test: http://ckeditor.t/tt/7153/2.html
comment:10 Changed 21 months ago by fredck
I would ask to please reconsider patch 1 as it has already been verified. Additionally, the provided tests are not working.
comment:11 Changed 21 months ago by garry.yao
R+ for the first patch, the extended tc opens #8285.
comment:12 Changed 21 months ago by garry.yao
- Status changed from review to review_passed
- Version set to 3.0
- Milestone set to CKEditor 3.6.2
comment:13 Changed 21 months ago by garry.yao
- Status changed from review_passed to closed
- Resolution set to fixed
Fixed with [7228].
