Ticket #4729 (closed New Feature: fixed)

Opened 4 months ago

Last modified 4 months ago

Support fakeobjects for comments

Reported by: fredck Owned by: fredck
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

4729.patch Download (11.6 KB) - added by fredck 4 months ago.
drupalbreaks.zip Download (2.7 KB) - added by fredck 4 months ago.
Drupal plugin using this feature. Updated to support tables, lists, blockquotes, etc.
4729_2.patch Download (12.2 KB) - added by fredck 4 months ago.
4729_3.patch Download (11.4 KB) - added by fredck 4 months ago.
4729_4.patch Download (11.3 KB) - added by fredck 4 months ago.

Change History

Changed 4 months ago by fredck

Changed 4 months ago by fredck

  • keywords Review? added
  • status changed from new to assigned

Changed 4 months ago by fredck

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 4 months ago by fredck

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

Changed 4 months ago by pwilk

  • cc pawel@… added
  • keywords Confirmed Review? removed

Changed 4 months ago by pwilk

  • keywords Confirmed Review? added

Changed 4 months 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 4 months ago by fredck

Changed 4 months ago by fredck

  • keywords Review? added; Review- removed

Changed 4 months 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.

Changed 4 months ago by garry.yao

  • keywords Review- added; Review? removed

Changed 4 months ago by fredck

Changed 4 months ago by fredck

  • keywords Review? added; Review- removed

Changed 4 months ago by fredck

Changed 4 months ago by fredck

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).

Changed 4 months ago by garry.yao

  • keywords Review+ added; Review? removed

Changed 4 months ago by fredck

  • status changed from assigned to closed
  • resolution set to fixed

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

Changed 4 months ago by wwalc

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

Note: See TracTickets for help on using tickets.