Ticket #8893 (closed Bug: fixed)

Opened 2 years ago

Last modified 16 months ago

Error in documentation or bug in PasteFromWordCleanupFile option

Reported by: mkesicki Owned by: Reinmar
Priority: Normal Milestone: CKEditor 4.0.1
Component: Core : Pasting Version: 3.1
Keywords: HasTest Cc:

Description (last modified by j.swiderski) (diff)

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

8893.patch (847 bytes) - added by j.swiderski 2 years ago.
8893_1.patch (2.5 KB) - added by j.swiderski 17 months ago.

Change History

comment:1 Changed 2 years ago by j.swiderski

  • Description modified (diff)

comment:2 Changed 2 years ago by j.swiderski

  • Status changed from new to confirmed
  • Version changed from 3.6.2 to 3.1

Reproducible from CKEditor 3.1.

comment:3 Changed 2 years ago by j.swiderski

  • Status changed from confirmed to assigned
  • Owner set to j.swiderski

Changed 2 years ago by j.swiderski

comment:4 Changed 2 years ago by j.swiderski

Assuming documentation is right.

  1. Problem: there is no default value assigned to CKEDITOR.config.pasteFromWordCleanupFile
  2. 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?

Last edited 22 months ago by j.swiderski (previous) (diff)

comment:5 Changed 2 years ago by j.swiderski

  • Status changed from assigned to review

comment:6 Changed 19 months ago by fredck

  • Status changed from review to 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 17 months ago by j.swiderski

comment:7 Changed 17 months ago by j.swiderski

  • Status changed from review_failed to 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'

Last edited 17 months ago by j.swiderski (previous) (diff)

comment:8 Changed 17 months ago by j.swiderski

#9718 was marked as duplicate.

comment:9 Changed 17 months ago by Reinmar

  • Status changed from review to review_failed
  • Milestone set to CKEditor 4.0.1

R-ed, because patch has to be ported to v4.

comment:10 Changed 17 months ago by Reinmar

  • Owner changed from j.swiderski to Reinmar
  • Status changed from review_failed to assigned

comment:11 Changed 17 months ago by Reinmar

  • Status changed from assigned to review

Opened git:3a543ae plus tests for review.

comment:12 Changed 17 months ago by Reinmar

  • Keywords HasTest added

comment:13 Changed 16 months ago by a.nowodzinski

  • Status changed from review to review_failed

comment:14 Changed 16 months ago by a.nowodzinski

Pastefromword requires a basic test for default filter configuration.

comment:15 Changed 16 months ago by Reinmar

  • Status changed from review_failed to review

Added basic test.

Force pushed both branches because of rebase.

comment:16 Changed 16 months ago by a.nowodzinski

  • Status changed from review to review_passed

comment:17 Changed 16 months ago by Reinmar

  • Status changed from review_passed to closed
  • Resolution set to fixed

Masterised git:894b1bc.

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