Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#4648 closed New Feature (fixed)

Support for iframe

Reported by: fredck Owned by: Saare
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 comp615 6 years ago.
iframe plugin (EN, DE)
4648.patch (60.2 KB) - added by comp615 6 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 Saare 6 years ago.
icons.png (5.9 KB) - added by Saare 6 years ago.
The new icon set image
4648_3.patch (14.0 KB) - added by Saare 6 years ago.
4648_4.patch (14.0 KB) - added by comp615 6 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 Saare 6 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 7 years ago by fredck

  • Milestone changed from CKEditor 3.2 to CKEditor 3.3

comment:2 Changed 7 years ago by alfonsoml

  • Milestone changed from CKEditor 3.3 to CKEditor 3.4

comment:3 Changed 7 years ago by brooks

  • Owner set to brooks
  • Status changed from new to assigned

comment:4 Changed 7 years ago by brooks

  • Owner brooks deleted
  • Status changed from assigned to new

comment:5 Changed 7 years ago by brooks

  • Owner set to brooks
  • Status changed from new to assigned

comment:6 Changed 6 years ago by brooks

  • Owner brooks deleted
  • Status changed from assigned to new

Changed 6 years ago by comp615

iframe plugin (EN, DE)

comment:7 Changed 6 years ago by comp615

  • Keywords HasPatch Review? added; Confirmed removed
  • Owner set to comp615
  • Status changed from new to assigned

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 6 years ago by comp615

  • Keywords Confirmed added

Changed 6 years ago by comp615

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 6 years ago by fredck

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 6 years ago by comp615

Done. Thanks!

comment:11 Changed 6 years ago by Saare

  • Keywords Review? removed
  • Owner changed from comp615 to Saare
  • Status changed from assigned to new
  • Version set to 3.0

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

Changed 6 years ago by Saare

Changed 6 years ago by Saare

The new icon set image

comment:12 Changed 6 years ago by Saare

  • 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 6 years ago by Saare

comment:13 Changed 6 years ago by Saare

  • Status changed from new to assigned

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

Changed 6 years ago by comp615

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

comment:14 Changed 6 years ago by fredck

  • Keywords HasPatch added; HasPatchConfirmed removed
  • Milestone changed from CKEditor 3.4 to CKEditor 3.5

comment:15 follow-up: Changed 6 years ago by comp615

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

comment:16 Changed 6 years ago by fredck

  • Milestone changed from CKEditor 3.4.1 to CKEditor 3.5

comment:17 in reply to: ↑ 15 Changed 6 years ago by comp615

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 6 years ago by garry.yao

  • Status changed from review to review_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 6 years ago by Saare

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 6 years ago by Saare

comment:20 Changed 6 years ago by Saare

  • Status changed from review_failed to review

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 6 years ago by garry.yao

  • Status changed from review to review_passed

Looks perfect, just waiting for the icons.

comment:22 Changed 6 years ago by Saare

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [5915].

comment:23 Changed 6 years ago by alfonsoml

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 6 years ago by garry.yao

Fixed with [6168].

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