Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#5654 closed New Feature (fixed)

Port 'Placeholder' to v3

Reported by: Garry Yao Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.5
Component: General Version:
Keywords: Cc:

Description

Placeholder is one FCKEditor 2.x plugin that introduce a way to present none-editable content in wysiwyg mode, similar with 'fakeobject' but allow dynamic content.

Attachments (8)

placeholder.gif (96 bytes) - added by Sa'ar Zac Elias 7 years ago.
5654.patch (6.9 KB) - added by Sa'ar Zac Elias 7 years ago.
5654_2.patch (10.0 KB) - added by Sa'ar Zac Elias 7 years ago.
5654_3.patch (10.2 KB) - added by Sa'ar Zac Elias 7 years ago.
5654_4.patch (10.2 KB) - added by Garry Yao 7 years ago.
5654_5.patch (10.0 KB) - added by Sa'ar Zac Elias 7 years ago.
5654_6.patch (10.5 KB) - added by Sa'ar Zac Elias 7 years ago.
5654_7.patch (10.2 KB) - added by Sa'ar Zac Elias 7 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 7 years ago by Garry Yao

Milestone: CKEditor 3.4CKEditor 3.5

comment:2 Changed 7 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: confirmedassigned

comment:3 Changed 7 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

Changed 7 years ago by Sa'ar Zac Elias

Attachment: placeholder.gif added

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654.patch added

comment:4 Changed 7 years ago by Sa'ar Zac Elias

Status: assignedreview

comment:5 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

Let's update the menu icon, and for plugins that does not get into core, we need a sample page for it.

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654_2.patch added

comment:6 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:7 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

This is the typical case of "pure plugin" implementation, so let's leave the lang entries into the plugin directory as well.

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654_3.patch added

comment:8 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

Changed 7 years ago by Garry Yao

Attachment: 5654_4.patch added

comment:9 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The way of how plugin language files loaded is wrong, the plugin system has already 'well established' infrastructure for load lang files, check 'uicolor' as an example.

Besides, double-click should simply opens the placeholder dialog.

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654_5.patch added

comment:10 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:11 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The 'doubleclick' processing is too simple that it doesn't fill up dialog correctly, in additional, please use the following lines as sample content:

<p>This is a [[sample placeholder]]. You are using <a href="http://ckeditor.com/">CKEditor</a>. </p>

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654_6.patch added

comment:12 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

The problem in fact was the detection of the element in the dialog.

comment:13 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

I think the 'onShow' logic is over complicated, how about:

				if ( isEdit )
				{
					var range = editor.getSelection().getRanges()[0];
					range.shrink( CKEDITOR.SHRINK_TEXT );
					var node = range.startContainer;
					while( node && !( node.type == CKEDITOR.NODE_ELEMENT && node.hasAttributes('_cke_placeholder') ) )
						node = node.getParent();
					this._element = node;
				}

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 5654_7.patch added

comment:14 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

As we talked, there is an unresloved edge case, but we can live with it for now.

comment:15 Changed 7 years ago by Garry Yao

Status: reviewreview_passed

comment:16 Changed 7 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [6051].

comment:17 Changed 7 years ago by Dinu

Reopen: accidental global var htmlFilter

plugin.js line 89: dataFilter = dataProcessor && dataProcessor.dataFilter; htmlFilter = dataProcessor && dataProcessor.htmlFilter;

There should be a comma instead of semicolon between them, or htmlFilter goes global.

comment:18 Changed 7 years ago by Sa'ar Zac Elias

Thanks, fixed as a micro change with [6093].

comment:19 Changed 7 years ago by Dinu

It's an easy hack to YUI Compiler to dump leaked vars, you could integrate in the build, especially easy since you have very few globals.

comment:20 in reply to:  19 Changed 7 years ago by Frederico Caldeira Knabben

Replying to dinu:

It's an easy hack to YUI Compiler to dump leaked vars, you could integrate in the build, especially easy since you have very few globals.

We use JavaScript Lint every once in a while, and always before producing releases. So this thing would get fixed. Thanks for the tip anyway.

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