Opened 13 years ago
Closed 12 years ago
#8893 closed Bug (fixed)
Error in documentation or bug in PasteFromWordCleanupFile option
| Reported by: | Michał | Owned by: | Piotrek Koszuliński |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.0.1 |
| Component: | Core : Pasting | Version: | 3.1 |
| Keywords: | HasTest | Cc: |
Description (last modified by )
According to: http://docs.cksource.com/ckeditor_api/symbols/CKEDITOR.config.html#.pasteFromWordCleanupFile it should works when I add
CKEDITOR.config.pasteFromWordCleanupFile = 'custom';
to config.js file, but it doesn't work. This works fine:
CKEDITOR.config.pasteFromWordCleanupFile = 'plugins/pastefromword/filter/custom.js';
Proposed fix to _source/plugins/pastefromword/plugin.js line 104 is change
var filterFilePath = CKEDITOR.getUrl( CKEDITOR.config.pasteFromWordCleanupFile || ( this.path + 'filter/default.js' ) );
to something like:
var filterFilePath = CKEDITOR.getUrl((this.path+CKEDITOR.config.pasteFromWordCleanupFile+'.js') || ( this.path + 'filter/default.js' ) );
or fix description in Documentation.
Attachments (2)
Change History (19)
comment:1 Changed 13 years ago by
| Description: | modified (diff) |
|---|
comment:2 Changed 13 years ago by
| Status: | new → confirmed |
|---|---|
| Version: | 3.6.2 → 3.1 |
comment:3 Changed 13 years ago by
| Owner: | set to Jakub Ś |
|---|---|
| Status: | confirmed → assigned |
Changed 13 years ago by
| Attachment: | 8893.patch added |
|---|
comment:4 Changed 13 years ago by
Assuming documentation is right.
- Problem: there is no default value assigned to
CKEDITOR.config.pasteFromWordCleanupFile - Problem: described method of just specifying file name placed in “filter” folder will not work.
The above patch solves both of these issues.
Only thing that I may have missed is that what if user assigns empty value to this property CKEDITOR.config.pasteFromWordCleanupFile = '' ? Shouldn't we handle this case as well?
comment:5 Changed 13 years ago by
| Status: | assigned → review |
|---|
comment:6 Changed 12 years ago by
| Status: | review → review_failed |
|---|
The proposed patch is wrong as it makes it impossible to have the file outside the editor installation. I believe this is the main intention with the original implementation.
Btw, the pasteFromWordCleanupFile DOES have a default value, which is "<plugin folder>/filter/default.js".
The thing to be fixed here is the documentation. There are other possibilities, but better to just KISS.
Changed 12 years ago by
| Attachment: | 8893_1.patch added |
|---|
comment:7 Changed 12 years ago by
| Status: | review_failed → review |
|---|
There was still one error in code. Value from configuration option was never read.
I have fixed it plus updated docs. I’m putting it on review again.
NOTE I'm not only sure if default value should be <plugin path> + 'filter/default.js' or 'plugins/pastefromword/filter/default.js'
comment:9 Changed 12 years ago by
| Milestone: | → CKEditor 4.0.1 |
|---|---|
| Status: | review → review_failed |
R-ed, because patch has to be ported to v4.
comment:10 Changed 12 years ago by
| Owner: | changed from Jakub Ś to Piotrek Koszuliński |
|---|---|
| Status: | review_failed → assigned |
comment:11 Changed 12 years ago by
| Status: | assigned → review |
|---|
Opened git:3a543ae plus tests for review.
comment:12 Changed 12 years ago by
| Keywords: | HasTest added |
|---|
comment:13 Changed 12 years ago by
| Status: | review → review_failed |
|---|
comment:14 Changed 12 years ago by
Pastefromword requires a basic test for default filter configuration.
comment:15 Changed 12 years ago by
| Status: | review_failed → review |
|---|
Added basic test.
Force pushed both branches because of rebase.
comment:16 Changed 12 years ago by
| Status: | review → review_passed |
|---|
comment:17 Changed 12 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Masterised git:894b1bc.
Reproducible from CKEditor 3.1.