Opened 11 years ago

Last modified 10 years ago

#10742 confirmed Task

CKEDITOR.style issues (documentation, usability, …)

Reported by: xmo Owned by:
Priority: Normal Milestone:
Component: Core : Styles Version:
Keywords: Cc:

Description

I've been looking into CKEDITOR.style in order to plug in custom and possibly somewhat involved styles. From the outset, and in combination with stylecombo, it seemed to strike a good balance between ease of customization and depth of said customization, a middle-ground between just customizing what controls are in the editor toolbar and creating completely new controls from scratch (or reimplementing the toolbar).

The main purpose at the moment is to integrate bootstrap and custom classes and have custom styles toggling these on the parent element (for collapsed selections, and ideally creating a new element with the right class for non-collapsed selections in the longer run), living in the standard toolbar's styles dropdown; and to replicate/merge the format dropdown there as well.

In the longer run, contextual enabling and disabling (possibly based on more complex predicates than just an element name) is expected.

In doing so, I've hit a bunch of snags:

Documentation

Styles are more or less undocumented: the API documentation is marked as a "work in progress" and mostly empty, and the only guide I've found is little more than a few examples of styleDescription.

Neither really explain the semantics of styles created this way (whether and how they are filtered or applied, how their removal works, …), the case of a STYLE_OBJECT type (or indeed the existence of the type attribute at all), that element can be an object (and that this radically changes the behavior of the style) or that the style's #element or #_.definition will be accessed directly for various reasons.

The guide seems somewhat better fleshed out in the 3.x documentation. Amongst other things it does mention the various style types (though not that they can be overridden, that element can be an object, or what the semantics of each type are; the list of block-level elements is also very incomplete compared to CKEditor 4's).

API

So far I've seen/had these issues with the CKEDITOR.style API, and its usage by other CKEDITOR code:

  • The purpose of some methods is unclear and I am not certain they are even called e.g. applyToRange and applyToObject seem to make sense (although applyToObject might be better called applyToElement, unless it's meant to apply only to STYLE_OBJECT elements? It seems to only ever be called by the div plugin, and I'm not sure of the context) but apply does not (it's redundant with Editor#applyStyle). Similarly, checkElementRemovable's purpose is unclear, as are its semantics in some corner-cases (e.g. if the style can *alter* one of the element's attributes but won't remove anythingd, should it return true or false?).
  • The main user of style objects (at least for my own use case), stylescombo, can only take a stylesDescription (or put differently stylesSet is always an array of styleDescription and can't trivially be a CKEDITOR.style or instance of a sub-type thereof) (there's a second issue with CKEDITOR.tools.clone being invoked on instanceConfig which also requires CKEDITOR.style to handle being CK-cloned). Without changing this, providing a custom CKEDITOR.style object (or subtype instance) can be fairly challenging (it's possible but hackish[0]). It also makes other very strange uses of style object methods, e.g. it has a special case in which it *never* calls buildPreview if the style's type is STYLE_OBJECT.
  • Most of the implementation of CKEDITOR.style is neither overridable (with a fine grain) nor directly accessible, makes direct access to CKEDITOR.style attributes rather than request services or behaviors from it. There are a number of accesses to #element and #._.definition (both #styles and #attributes) outside of CKEDITOR.style (constructor or prototype) meaning custom styles (or CKEDITOR.style subtypes) have to remain very close to the original *or* fully reimplement everything (at least the "entry points" seem respected though I have not yet dived into that). Considering the complexity of of the most useful utility functions (e.g. applyInlineStyle) this is quite bothersome for the implementation of a custom style type as a pretty significant amount of work must be duplicated.
  • The one and only accessible utility function is (oddly enough) CKEDITOR.style.getStyleText, and it's a "class" function on CKEDITOR.style which means it can't easily be fixed up by-style when the existing version makes little sense for a specific custom style or style type (basically precluding the usage of its callers in a custom tyle type)
  • The built-in style type has no support for toggling classes on existing elements (it treats class as any other element to set/unset) and because the preview dropdown (of stylecombo) is a separate iframe it's also impossible to apply class styles to the preview (even if one manages to find out that TYPE_OBJECT is not doing to allow previews at all)

Conclusion

I'm posting this more as a task, as it's not exactly a bug nor is it precisely a request. Repeating the introduction and having spent a few days with it, I still think CKEDITOR.style has quite a bit of potential for CKEDITOR integrators, but not in its current shape, and thus would like to start a discussion on it.

[0] essentially the requirements are to 1. replace CKEDITOR.style by a sub-type copying getStyleText and altering the constructor to return the first parameter directly if it's already an instance of CKEDITOR.style and 2. add a constructor property which returns the object it's called on instead of creating a new one.

Change History (2)

comment:1 Changed 11 years ago by Piotrek Koszuliński

Status: newconfirmed

Thanks for the feedback. I haven't read it yet, but we are thinking about improving styles system (which I also find too limited), so your opinion will be then very valuable.

comment:2 Changed 11 years ago by Jakub Ś

Other issues can be found in #10824, #10675, #4505, #5980 #11120.

Last edited 10 years ago by Jakub Ś (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy