Ticket #5024 (closed Task: fixed)

Opened 5 years ago

Last modified 4 years ago

Port doctype samples from FCKeditor

Reported by: alfonsoml Owned by: Saare
Priority: Normal Milestone: CKEditor 3.4.2
Component: General Version: 3.0
Keywords: Cc:

Description

Port the samples 14, 15 and 16 to show how to generate XHTML, HTML 4 or legacy HTML for flash.

Attachments

5024.patch (3.0 KB) - added by alfonsoml 5 years ago.
Improved documentation
xhtml.html (6.0 KB) - added by alfonsoml 5 years ago.
Proposed XHTML sample
5024_2.patch (9.7 KB) - added by alfonsoml 5 years ago.
Updated XHTML sample
5024_3.patch (10.1 KB) - added by alfonsoml 5 years ago.
Revised patch
5024_4.patch (9.7 KB) - added by alfonsoml 4 years ago.
Revised patch
5024_5.patch (7.6 KB) - added by alfonsoml 4 years ago.
HTML sample
5024_6.patch (8.5 KB) - added by alfonsoml 4 years ago.
Revised patch
5024_7.zip (32.5 KB) - added by alfonsoml 4 years ago.
Flash sample
5024_8.rar (36.6 KB) - added by Saare 4 years ago.
5024_9.zip (40.8 KB) - added by Saare 4 years ago.

Change History

Changed 5 years ago by alfonsoml

Improved documentation

Changed 5 years ago by alfonsoml

Proposed XHTML sample

comment:1 Changed 5 years ago by alfonsoml

  • Keywords Confirmed Review? added

The 5024.html file is documentation for some of the options that were missing and I needed to find.

The sample file it's a port from FCKeditor. It has three problems #5150, #5152 and using the SVN, the call to CKEDITOR.stylesSet.add or CKEDITOR.addStylesSet inline fails. In that aspect, I think that when #4973 is applied to the release version, the people that have CKEDITOR.addStylesSet calls inline will get an error, but that's for another day. I need some rest.

The review is mainly for the doc patches as I might have made some mistake, but any comment about the sample is welcome.

comment:2 Changed 5 years ago by fredck

  • Milestone set to CKEditor 3.3

comment:3 Changed 5 years ago by alfonsoml

  • Keywords Review? removed

Documentation added with [5155]

Changed 5 years ago by alfonsoml

Updated XHTML sample

comment:4 Changed 5 years ago by alfonsoml

  • Owner set to alfonsoml
  • Keywords Review? added
  • Status changed from new to assigned

The new XHTML sample should work properly after applying #5150 and #5152

comment:5 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Combo items are not correctly styled.

comment:6 Changed 5 years ago by alfonsoml

  • Keywords Review? added; Review- removed

Filed #5292 for that problem

In the new patch I'm using the pluginsLoaded event to add the stylesSet data correctly.

Changed 5 years ago by alfonsoml

Revised patch

comment:7 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Let's exclude the style preview problem which could be later fixed by #5292 instead.
I'm still confused about the reason why keeping 'my_styles' in the main file? Considering the fact that 'pluginsLoaded' stuff is a hack, we should not have it appear in such a sample page.

comment:8 Changed 5 years ago by alfonsoml

  • Keywords Review? added; Review- removed

The idea is to show how to define the styles without an extra file/request. The problem is that using the _source files if we put the CKEDITOR.stylesSet call in the main script it will generate an error because the file hasn't been loaded at that moment.

That won't happen in the release version, but we need to test it in _source so I thought to put it this way. Do you have another idea? I think that it's interesting to provide the full sample with some custom styles.

comment:9 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

If the possibility of adding combo styles as literal is important, then we should think about duck typing 'CKEDITOR.config.stylesCombo_stylesSet':

/**
 * The "styles definition set" to load into the styles combo. The styles may
 * be defined in the page containing the editor, or can be loaded on demand
 * from an external file when opening the styles combo for the fist time. In
 * the second case, if this setting contains only a name, the styles definition
 * file will be loaded from the "styles" folder inside the stylescombo plugin
 * folder. Otherwise, this setting has the "name:url" syntax, making it
 * possible to set the URL from which loading the styles file.
 * @type String|Array
 * @default 'default'
 * @example
 * // Load from the stylescombo styles folder (mystyles.js file).
 * config.stylesCombo_stylesSet = 'mystyles';
 * @example
 * // Load from a relative URL.
 * config.stylesCombo_stylesSet = 'mystyles:/editorstyles/styles.js';
 * @example
 * // Load from a full URL.
 * config.stylesCombo_stylesSet = 'mystyles:http://www.example.com/editorstyles/styles.js';
 * @example
 * // Load from a list of definitions.
 * config.stylesCombo_stylesSet = [
 *  { name : 'Strong Emphasis', element : 'strong' },
 * { name : 'Emphasis', element : 'em' }, ... ];
 */
CKEDITOR.config.stylesCombo_stylesSet = 'default';

Changed 4 years ago by alfonsoml

Revised patch

comment:10 Changed 4 years ago by alfonsoml

  • Keywords Review? added; Review- removed

That's a good idea because it's simple and matches the behavior of the toolbar. I've created #5352 for it and the new patch here uses that approach.

comment:11 Changed 4 years ago by garry.yao

Depend on #5352.

comment:12 Changed 4 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:13 Changed 4 years ago by alfonsoml

  • Keywords Review+ removed

XHTML sample included with [5302]

Changed 4 years ago by alfonsoml

HTML sample

comment:14 Changed 4 years ago by alfonsoml

  • Keywords Review? added

The new sample shows how to use fonts and output images with dimensions as attributes instead of styles (some mails don't accept styles)

comment:15 Changed 4 years ago by garry.yao

  • Keywords Review- added; Review? removed

Color values in RGB doesn't get converted into RRGGBB on attribute thus font fore/back colors doesn't display in IE.

comment:16 follow-up: ↓ 20 Changed 4 years ago by alfonsoml

That problem is #1653. I'll try to fix it.

Changed 4 years ago by alfonsoml

Revised patch

comment:17 Changed 4 years ago by alfonsoml

  • Keywords Review? added; Review- removed

I've included in the sample the function to convert rgb() colors so we can publish the sample without further delays.

comment:18 Changed 4 years ago by fredck

  • Keywords Review+ added; Review? removed

Well... I think people will not really use this one as a sample, but as a template for code snippet, because it looks so complex. But anyway, some people may find it useful.

comment:19 Changed 4 years ago by garry.yao

  • Keywords Review- added; Review+ removed

The problem is still there, when applying the 'Red' font color.

comment:20 in reply to: ↑ 16 Changed 4 years ago by garry.yao

Replying to alfonsoml:

That problem is #1653. I'll try to fix it.

Consistency output of color pattern is not a priority now.

comment:21 Changed 4 years ago by alfonsoml

  • Keywords Review? added; Review- removed

Fixed RGB issues with #5676. I did misunderstood the previous comments.

comment:22 Changed 4 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:23 Changed 4 years ago by alfonsoml

  • Keywords Review+ removed
  • Milestone changed from CKEditor 3.3 to CKEditor 3.4

HTML sample added with [5491]. Flash for the next version.

comment:24 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.4 to CKEditor 3.5

comment:25 Changed 4 years ago by fredck

  • Keywords Confirmed removed

Changed 4 years ago by alfonsoml

Flash sample

comment:26 Changed 4 years ago by alfonsoml

  • Status changed from assigned to review

I had the Flash sample ready since some time ago, but it was pending to change the graphics in the flash file to put CKEditor instead of FCKeditor. I've realized that it could take too long for me to install the flash compiler just for this task, so I'm asking for review of the sample itself and hope that someone else can change the .fla and create a new .swf file.

Changed 4 years ago by Saare

comment:27 Changed 4 years ago by Saare

Updated the logo.

comment:28 Changed 4 years ago by garry.yao

  • Status changed from review to review_failed

'Send to Flash' doesn't work for me.

comment:29 Changed 4 years ago by alfonsoml

Yes, the new swf seems to be missing something because it doesn't work, but the previous version works correctly.

Changed 4 years ago by Saare

comment:30 Changed 4 years ago by Saare

Let's try this one.

comment:31 Changed 4 years ago by garry.yao

  • Status changed from review_failed to review
  • Owner changed from alfonsoml to garry.yao

comment:32 Changed 4 years ago by garry.yao

  • Status changed from review to review_passed

Nice done, only to fix the typo from index.html to 'Output for Flash'.

comment:33 Changed 4 years ago by Saare

  • Owner changed from garry.yao to Saare
  • Status changed from review_passed to assigned

comment:34 Changed 4 years ago by Saare

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

Fixed with [5983].

comment:35 Changed 4 years ago by Saare

Changelog entry added with [5984].

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