Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4729 closed New Feature (fixed)

Support fakeobjects for comments

Reported by: Frederico Caldeira Knabben Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.1
Component: General Version:
Keywords: Confirmed Review+ Cc: pawel@…

Description

Currently, it's not possible to replace HTML comments with fake elements. We should provide support for it.

Attachments (5)

4729.patch (11.6 KB) - added by Frederico Caldeira Knabben 8 years ago.
drupalbreaks.zip (2.7 KB) - added by Frederico Caldeira Knabben 8 years ago.
Drupal plugin using this feature. Updated to support tables, lists, blockquotes, etc.
4729_2.patch (12.2 KB) - added by Frederico Caldeira Knabben 8 years ago.
4729_3.patch (11.4 KB) - added by Frederico Caldeira Knabben 8 years ago.
4729_4.patch (11.3 KB) - added by Frederico Caldeira Knabben 8 years ago.

Download all attachments as: .zip

Change History (18)

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 4729.patch added

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Status: newassigned

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

The motivation to code this feature right now is the support for break and pagebreak comments in Drupal. I've attached a proposal plugin for a Drupal plugin for it, which uses the features introduced in the proposed patch.

To activate it for testing, after applying the patch, simply unzip the file into the plugins folder and use the following config.js contents:

CKEDITOR.editorConfig = function( config )
{
	config.extraPlugins = 'drupalbreaks';
	
	this.on( 'pluginsLoaded', function()
	{
		CKEDITOR.config.toolbar_Full.push( [ 'DrupalBreak', 'DrupalPageBreak' ] );
	});
};

The basic idea is that we must always work to make plugins development a simply and exciting task. For that, our API must be flexible and all complexity must be handled by the core code.

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: drupalbreaks.zip added

Drupal plugin using this feature. Updated to support tables, lists, blockquotes, etc.

comment:3 Changed 8 years ago by Paweł Wilk

Cc: pawel@… added
Keywords: Confirmed Review? removed

comment:4 Changed 8 years ago by Paweł Wilk

Keywords: Confirmed Review? added

comment:5 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Nice patch, kudo!
And here come some small problems that I found:

L18-L42 of fakeobject/plugin.js

There should be some guard since we should also considering non-element fake now. ( Identified by resize the fake comment and switch to source you'll get a JavaScript error);

L114 of the same plugin

if ( fakeElement.getAttribute( '_cke_real_node_type' ) == CKEDITOR.NODE_COMMENT )
	return null;

How about

if ( fakeElement.getAttribute( '_cke_real_node_type' ) != CKEDITOR.NODE_ELEMENT )
	return null;

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 4729_2.patch added

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

comment:7 Changed 8 years ago by Garry Yao

Note that the priority changes to the default rules may have impact on existing plugins which may assume the default rules are applied in prior, if it's not a necessary to have it, we can hold this change.

comment:8 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 4729_3.patch added

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 4729_4.patch added

comment:10 Changed 8 years ago by Frederico Caldeira Knabben

Added a new patch based on a chat with Garry, where he stated that it's not a good idea to have the getText method into CKEDITOR.dom.comment, as this method is been (improperly) used in the code to detect node types (the type property should be used instead).

comment:11 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:12 Changed 8 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [4556] (3.1.x branch).

comment:13 Changed 8 years ago by Wiktor Walc

Added missing entry in ckeditor.pack with [4558].

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