Opened 10 years ago

Closed 10 years ago

Last modified 10 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 10 years ago.
5654.patch (6.9 KB) - added by Sa'ar Zac Elias 10 years ago.
5654_2.patch (10.0 KB) - added by Sa'ar Zac Elias 10 years ago.
5654_3.patch (10.2 KB) - added by Sa'ar Zac Elias 10 years ago.
5654_4.patch (10.2 KB) - added by Garry Yao 10 years ago.
5654_5.patch (10.0 KB) - added by Sa'ar Zac Elias 10 years ago.
5654_6.patch (10.5 KB) - added by Sa'ar Zac Elias 10 years ago.
5654_7.patch (10.2 KB) - added by Sa'ar Zac Elias 10 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 10 years ago by Garry Yao

Milestone: CKEditor 3.4CKEditor 3.5

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

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

comment:3 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

Changed 10 years ago by Sa'ar Zac Elias

Attachment: placeholder.gif added

Changed 10 years ago by Sa'ar Zac Elias

Attachment: 5654.patch added

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

Status: assignedreview

comment:5 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 5654_2.patch added

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

Status: review_failedreview

comment:7 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 5654_3.patch added

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

Status: review_failedreview

Changed 10 years ago by Garry Yao

Attachment: 5654_4.patch added

comment:9 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 5654_5.patch added

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

Status: review_failedreview

comment:11 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 5654_6.patch added

comment:12 Changed 10 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 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 5654_7.patch added

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

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Fixed with [6051].

comment:17 Changed 10 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 10 years ago by Sa'ar Zac Elias

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

comment:19 Changed 10 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 10 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 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy