Opened 10 years ago

Closed 10 years ago

Last modified 10 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 10 years ago.
drupalbreaks.zip (2.7 KB) - added by Frederico Caldeira Knabben 10 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 10 years ago.
4729_3.patch (11.4 KB) - added by Frederico Caldeira Knabben 10 years ago.
4729_4.patch (11.3 KB) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (18)

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 4729.patch added

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Status: newassigned

comment:2 Changed 10 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 10 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 10 years ago by Paweł Wilk

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

comment:4 Changed 10 years ago by Paweł Wilk

Keywords: Confirmed Review? added

comment:5 Changed 10 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 10 years ago by Frederico Caldeira Knabben

Attachment: 4729_2.patch added

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

comment:7 Changed 10 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 10 years ago by Garry Yao

Keywords: Review- added; Review? removed

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 4729_3.patch added

comment:9 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 4729_4.patch added

comment:10 Changed 10 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 10 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:12 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

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

comment:13 Changed 10 years ago by Wiktor Walc

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

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