Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#4648 closed New Feature (fixed)

Support for iframe

Reported by: Frederico Caldeira Knabben Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.5
Component: General Version: 3.0
Keywords: HasPatch Cc:

Description

The editor should provide support for iframes, just like it support object elements. A placeholder should be displayed for iframes. This ticket is not related to the dialog we could have to edit iframe properties, which should be a successive evolution for this feature.

Attachments (7)

iframe.zip (6.7 KB) - added by Charlie 10 years ago.
iframe plugin (EN, DE)
4648.patch (60.2 KB) - added by Charlie 10 years ago.
If you're lucky, this might work! The plugin would still need to be added to the extraPlugin list and toolbar (But I figured this wasn't my place to do). Someone will need to help proof over the final things like the images and such. (I couldn't get the toolbar image working from SVN). Hope this helps!
4648_2.patch (19.3 KB) - added by Sa'ar Zac Elias 10 years ago.
icons.png (5.9 KB) - added by Sa'ar Zac Elias 10 years ago.
The new icon set image
4648_3.patch (14.0 KB) - added by Sa'ar Zac Elias 10 years ago.
4648_4.patch (14.0 KB) - added by Charlie 10 years ago.
Fixed a minor bug where in IE, the window wasn't large enough. Increased minHeight to 260.
4648_5.patch (14.1 KB) - added by Sa'ar Zac Elias 10 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 11 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

comment:2 Changed 11 years ago by Alfonso Martínez de Lizarrondo

Milestone: CKEditor 3.3CKEditor 3.4

comment:3 Changed 10 years ago by brooks

Owner: set to brooks
Status: newassigned

comment:4 Changed 10 years ago by brooks

Owner: brooks deleted
Status: assignednew

comment:5 Changed 10 years ago by brooks

Owner: set to brooks
Status: newassigned

comment:6 Changed 10 years ago by brooks

Owner: brooks deleted
Status: assignednew

Changed 10 years ago by Charlie

Attachment: iframe.zip added

iframe plugin (EN, DE)

comment:7 Changed 10 years ago by Charlie

Keywords: HasPatch Review? added; Confirmed removed
Owner: set to Charlie
Status: newassigned

Added a plugin to do just this. Will re-code the language files and all shortly to create a patch file from SVN.

IMPORTANT: I really just scrapped together a placeholder image and toolbar icon. I don't know what the standards and such are on this, so someone might want to replace them...

comment:8 Changed 10 years ago by Charlie

Keywords: Confirmed added

Changed 10 years ago by Charlie

Attachment: 4648.patch added

If you're lucky, this might work! The plugin would still need to be added to the extraPlugin list and toolbar (But I figured this wasn't my place to do). Someone will need to help proof over the final things like the images and such. (I couldn't get the toolbar image working from SVN). Hope this helps!

comment:9 Changed 10 years ago by Frederico Caldeira Knabben

Hey comp615! Thanks for this nice contribution ;)

To be able to use your code, we need you to "electronically sign" our CLA. Could you please do so? You will find the instructions in the page. Thanks!

comment:10 Changed 10 years ago by Charlie

Done. Thanks!

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

Keywords: Review? removed
Owner: changed from Charlie to Sa'ar Zac Elias
Status: assignednew
Version: 3.0

Thanks @comp615! I'll review your code and upload a complete patch based on yours.

Changed 10 years ago by Sa'ar Zac Elias

Attachment: 4648_2.patch added

Changed 10 years ago by Sa'ar Zac Elias

Attachment: icons.png added

The new icon set image

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

Keywords: Review? added

I've been using the same images @comp615 made, but it's up to Fredrico to decide whether to use them or not. The fakeobject image can be found in the ZIP file attached by @comp615, and I attached the new icon set image.

Changed 10 years ago by Sa'ar Zac Elias

Attachment: 4648_3.patch added

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

Status: newassigned

I accidentally uploaded the wrong file previously, please ignore 4648_2.patch.

Changed 10 years ago by Charlie

Attachment: 4648_4.patch added

Fixed a minor bug where in IE, the window wasn't large enough. Increased minHeight to 260.

comment:14 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: HasPatch added; HasPatchConfirmed removed
Milestone: CKEditor 3.4CKEditor 3.5

comment:15 Changed 10 years ago by Charlie

note that minWidth should be set to 350 (Found bug in quirks mode)

comment:16 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

comment:17 in reply to:  15 Changed 10 years ago by Charlie

Replying to comp615:

note that minWidth should be set to 350 (Found bug in quirks mode)

This comment can be disregarded pending the outcome of #6076, or it can be made bigger anyways :)

comment:18 Changed 10 years ago by Garry Yao

Status: reviewreview_failed

It looks good, we could even make the dialog size justification later, but maybe two other important field which are 'name' and 'align' are missing? Besides, the place holder image has to be compatible with 'Flash' one.

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

How about using the dialogadvtab plugin in this dialog as well? We could remove the 'id', 'class' and 'style' fields and put 'name' and 'align' instead, and the removed fields would be placed in the tab generated by the dialogadvtab plugin.

Changed 10 years ago by Sa'ar Zac Elias

Attachment: 4648_5.patch added

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

Status: review_failedreview

I'm not able to create new images right now (icon strips and the placeholder image). It would be great if someone else could.

comment:21 Changed 10 years ago by Garry Yao

Status: reviewreview_passed

Looks perfect, just waiting for the icons.

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

Resolution: fixed
Status: review_passedclosed

Fixed with [5915].

comment:23 Changed 10 years ago by Alfonso Martínez de Lizarrondo

The plugin has been included in the default config.plugins, but it isn't packed into ckeditor.js so it creates an extra request for it.

It just needs an entry in ckeditor.pack, but currently svn is failing for me.

comment:24 Changed 10 years ago by Garry Yao

Fixed with [6168].

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