Opened 8 years ago

Closed 7 years ago

Last modified 7 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 7 years ago.
iframe plugin (EN, DE)
4648.patch (60.2 KB) - added by Charlie 7 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 7 years ago.
icons.png (5.9 KB) - added by Sa'ar Zac Elias 7 years ago.
The new icon set image
4648_3.patch (14.0 KB) - added by Sa'ar Zac Elias 7 years ago.
4648_4.patch (14.0 KB) - added by Charlie 7 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 7 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

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

Milestone: CKEditor 3.3CKEditor 3.4

comment:3 Changed 7 years ago by brooks

Owner: set to brooks
Status: newassigned

comment:4 Changed 7 years ago by brooks

Owner: brooks deleted
Status: assignednew

comment:5 Changed 7 years ago by brooks

Owner: set to brooks
Status: newassigned

comment:6 Changed 7 years ago by brooks

Owner: brooks deleted
Status: assignednew

Changed 7 years ago by Charlie

Attachment: iframe.zip added

iframe plugin (EN, DE)

comment:7 Changed 7 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 7 years ago by Charlie

Keywords: Confirmed added

Changed 7 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 7 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 7 years ago by Charlie

Done. Thanks!

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

Attachment: 4648_2.patch added

Changed 7 years ago by Sa'ar Zac Elias

Attachment: icons.png added

The new icon set image

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

Attachment: 4648_3.patch added

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

Status: newassigned

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

Changed 7 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 7 years ago by Frederico Caldeira Knabben

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

comment:15 Changed 7 years ago by Charlie

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

comment:16 Changed 7 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

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

Attachment: 4648_5.patch added

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

Status: reviewreview_passed

Looks perfect, just waiting for the icons.

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

Resolution: fixed
Status: review_passedclosed

Fixed with [5915].

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

Fixed with [6168].

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