Div dialog 'Style' select field in vain

[Garry Yao]

The 'elementStyle' field in div dialog have no effect right now, it will never be populated, we should have the functionality properly align with v2.

[Garry Yao]

In V2 we've been using "any customized styleset that apply to div' for it, while we don't have global styleset anymore, is it a good idea to just allow all block styles here so the 'div' element could even get chance to be changed to other blocks?

[Wiktor Walc]

We shouldn't take styles for all block elements, for example "div" should not become "ul" automatically if someone have defined style for the "ul" element. It is also uncommon to use headers as containers.

If we're going to provide styles for other elements than div and then change div into other element, the 'div' dialog should be renamed to 'container', a more generic name, otherwise the behaviour would be a little bit counterintuitive (after all it's a "div" dialog).

When no styles are available, we could disable that select field, to not confuse users and also we could provide some default style just to show users that such functionality exists (otherwise it will be disabled by default).

[Frederico Caldeira Knabben]

Replying to garry.yao:

In V2 we've been using "any customized styleset that apply to div' for it, while we don't have global styleset anymore, is it a good idea to just allow all block styles here so the 'div' element could even get chance to be changed to other blocks?

No, never. The <div> here is not a normal block, like a <p>. It's a container for other block elements. We must just replicated the V2 behavior.

[Garry Yao]

Modified Default stylesSet for Testing Div Dialog

[Garry Yao]

Alignment with the v2 feature by globalize the custom style definitions.
The attached 'default' stylesSet could be used for testing.

[Garry Yao]

The new patch handles the possible conflicts of same attributes/styles between the div styles and default dialog fields, please note that it's a bit counterintuitive that by setting 'Div Style' to empty on an existed stylish div will not actually remove that style.

[Alfonso Martínez de Lizarrondo]

I've created #5023 to improve the default styleSet sample.

[Alfonso Martínez de Lizarrondo]

The change in resourcemanager.js is breaking my test page where I load a plugin from a custom folder.

	CKEDITOR.plugins.addExternal( 'easyupload', '/CKEplugins/easyupload/' );

	config.extraPlugins='easyupload';

now tries to load

    '/CKEplugins/easyupload/undefined?t=1263666437818'

After fixing that, what about changing

CKEDITOR.stylesSet.addExternal( styleSetName,
		externalPath ? stylesSet.slice( 1 ).join( ':' ) : pluginPath + 'styles/',
		externalPath ? '' : styleSetName + '.js' );

to

CKEDITOR.stylesSet.addExternal( styleSetName,
		externalPath ? stylesSet.slice( 1 ).join( ':' ) : pluginPath + 'styles/' + styleSetName + '.js', 
		'' );

I would move all that initialization (lines 202-209) to the top of the init function of the plugin (line 18)

Please, read #5025 with regards to the removal of two API functions.

In the dialog, line 453 doesn't need the assignment to the 'style' var, and so that var can be removed also from line 443

    style = styles[ styleName ] = new CKEDITOR.style( styleDefinition ); 

The rest seems to work, although I haven't tried any kind of extensive check.

[Garry Yao]

New patch targeting all above comments.

[Alfonso Martínez de Lizarrondo]

This patch creates a bug in IE:

1 Load replacebyclass in IE8 2 Select the contents (or part of it) and create a DIV container. No need to specify any attribute

3. Edit the container using the context menu. Don't change anything and click OK
4. Switch to source and back to design
5. Try to edit again the container like in step 3. It gives a "permission denied" at line 97 in a call from commitInternally

This one can be filed as another bug as it also happens in 3.1: trying to set the content direction to LTR initially fails, it's set only after setting first RTL.

[Garry Yao]

New patch targeting the following issues:

1. 'CKEDITOR.stylesSet' should defined by styles plugin instead of stylesCombo plugin;
2. Div dialog should disable empty styles field instead of hiding it;
3. IE script error when open Div dialog for editing.

[Garry Yao]

I'm committing this as a new feature since it's actually an alignment with v2.
Fixed with [5050] on 3.2.x branch.