Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3673 closed New Feature (fixed)

Add file browser support to CKEditor

Reported by: Wiktor Walc Owned by: Wiktor Walc
Priority: Normal Milestone: CKEditor 3.0
Component: File Browser Version:
Keywords: Confirmed 3.0RC Review+ Cc:

Description

To do: create a plugin that makes it possible to integrate any file browser in CKEditor (it will not provide any file browser solution).

Attachments (12)

3673.patch (25.1 KB) - added by Wiktor Walc 8 years ago.
3673_2.patch (25.4 KB) - added by Wiktor Walc 8 years ago.
3673_3.patch (24.1 KB) - added by Wiktor Walc 8 years ago.
3673_4.patch (24.2 KB) - added by Wiktor Walc 8 years ago.
added missing language entry
3673_5.patch (29.4 KB) - added by Wiktor Walc 8 years ago.
3673_6.patch (31.0 KB) - added by Wiktor Walc 8 years ago.
3673_7.patch (17.4 KB) - added by Martin Kou 8 years ago.
3673_8.patch (32.2 KB) - added by Martin Kou 8 years ago.
3673_9.patch (30.2 KB) - added by Wiktor Walc 8 years ago.
3673_10.patch (29.0 KB) - added by Wiktor Walc 8 years ago.
3673_11.patch (27.3 KB) - added by Wiktor Walc 8 years ago.
3673_12.patch (26.7 KB) - added by Wiktor Walc 8 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 8 years ago by Wiktor Walc

Owner: set to Wiktor Walc

comment:2 Changed 8 years ago by Artur Formella

Isn't it a DUP of #2873

comment:3 Changed 8 years ago by Wiktor Walc

Good question - no it isn't.

This ticket is abot a plugin that adds support for all possible file browsers to CKEditor (built-in file browser, CKFinder and other), just like it is now possbile to integrate external file browsers in FCKeditor using LinkBrowserURL etc. settings.

In #2873 a basic file browser is to be created.

comment:4 Changed 8 years ago by Artur Formella

Oh. This is a very good idea:)

comment:5 Changed 8 years ago by Artur Formella

#3682 has been marked as DUP.

Changed 8 years ago by Wiktor Walc

Attachment: 3673.patch added

comment:6 Changed 8 years ago by Wiktor Walc

Keywords: Review? added

I'm not sure what to do with CKEDITOR.config.image_uploadAction / linkUploadAction - I think we may simply drop these values.

It would be nice to have a throbber during quick uploads.

To use CKFinder with CKEditor, set filebrowser_browse and filebrowser_upload settings:

CKEDITOR.replace( 'editor1',
{
	filebrowser_browse_url : '/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl',
	filebrowser_image_browse_url : '/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl&Type=Images',
	filebrowser_flash_browse_url : '/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl&Type=Flash',
	filebrowser_upload_url : '/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Files',
	filebrowser_image_upload_url : '/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Images',
	filebrowser_flash_upload_url : '/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Flash'
} );

In english language file there are some predefined error codes. It is also possible to pass the localized error message to CKEditor instead of error code, that's why active language code in CKEditor is appended automatically to urls.

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I've talked with Wiktor and we've found out that some simplifications could be done to the code. A new patch will be coming for it.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_2.patch added

comment:8 Changed 8 years ago by Wiktor Walc

Keywords: Review? added; Review- removed
  • filebrowser can be set to 'true', 'elementId', 'tabId:elementId' or an object
  • removed error messages from language file
  • changed configuration settings to camelCase format
  • QuickUpload and Browse commands expects the SetUrl function to be called by default
  • removed image_uploadAction/flashUploadAction settings. (btw. which name is correct? - config.plugin_settingName vs config.pluginSettingName - we are using both versions right now, for example config.toolbarCanCollapse and config.smiley_columns)
  • filebrowser + popup plugins take 4KB after compression
  • apart from 'langCode=en', another extra parameter is passed to the connector: CKEditor=1. This way server connector can handle requests coming from CKEditor in a special way.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_3.patch added

Changed 8 years ago by Wiktor Walc

Attachment: 3673_4.patch added

added missing language entry

Changed 8 years ago by Wiktor Walc

Attachment: 3673_5.patch added

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The functionNumber and editorInstance are being improperly assigned. They will not handle more than one editor instance in the page.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_6.patch added

Changed 8 years ago by Martin Kou

Attachment: 3673_7.patch added

Changed 8 years ago by Martin Kou

Attachment: 3673_8.patch added

comment:10 Changed 8 years ago by Martin Kou

Added missing files.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_9.patch added

comment:11 Changed 8 years ago by Wiktor Walc

Keywords: Review? added; Review- removed

Yet another patch, this time it should work even if there are multiple instances of CKEditor on the same page. Patch for #3758 included for easier testing.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_10.patch added

comment:12 Changed 8 years ago by Wiktor Walc

In 3673_10.patch I have removed patch for #3758, a valid patch for #3758 should be applied before testing 3673_10.patch.

comment:13 Changed 8 years ago by Martin Kou

Keywords: Review- added; Review? removed

I can't really test for the correctness of the new plugin because there's no back end for the file browsers. So I can only look for unclean things in the code.

  1. getParentTab() and selectParentTab() in dialog plugin.js can be consolidated - currently getParentTab() is just copying code from selectParentTab() and that's not good.
  2. L44,45 and L50,51 in link dialog can be consolidated - doing the same thing at the end of both if and else means you can just do it once after the whole if block.
  3. L308 - L318 in filebrowser plugin.js - can't we just force all filebrowser values to be page_id:element_id, which is much faster and less error prone?
  4. L1219 in dialogui plugin.js - how about haivng a setAction() instead? Having an optional action in reset() makes the code harder to understand.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_11.patch added

comment:14 Changed 8 years ago by Wiktor Walc

Keywords: Review? added; Review- removed

Removing 3) and supporting just the page_id:element_id format solved 1) - it is not needed anymore.

4) It seems that since the first patch something has changed and now the form action is remembered correctly even if the action was set later by the filebrowser plugin, removed.

2) added new variable.

Changed 8 years ago by Wiktor Walc

Attachment: 3673_12.patch added

comment:15 Changed 8 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:16 Changed 8 years ago by Wiktor Walc

Resolution: fixed
Status: newclosed

Fixed with [3716].

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