Incorrect element wrapping when coreStyles_bold is configured to use divs

[sarahjones]

Descriptive summary - Using the coreStyles_bold editor config option does not put the bold tag in the correct place and inserts the attributes (for bolding) on the wrong tag.

Steps to reproduce -

1) Set editor configuration to include:

enterMode: CKEDITOR.ENTER_DIV,
coreStyles_bold: {
      element: 'div',
      attributes: { class: 'bold' }
    }

2) Open the editor 3) Type some text 4) Highlight part of the text (best seen if you highlight the middle of a word) 5) Click the editor bold button

For the Observed and Desired, the text is "The is my bolded text" where "olde" was the highlighted text when pressing the bold button

Observed: The whole line of text is bolded and the HTML looks like

<div class="bold">The is my bolded text</div>

Desired: Only the highlighted text is bolded and the HTML looks like

<div>The is my b<div class="bold">olde</div>d text</div>

Browser and OS - OS X 10.9.3; Firefox 33.1 and Chrome 40.0.2214.93 (64-bit)

Screenshot - Please see attached

Sample data - This bug is not correlated with specific data

Test case file - Please see ​https://github.com/sarahjones/ckeditor_bold_bug for an isolated setup. Please see index.html (all customized config is there)

Build configuration - This is using the 4.47 (ckeditor_4.4.7_9e2f4050d27c.zip) Standard package.

[Jakub Ś]

Let me start by saying that this issue is invalid and behaviour is correct.

First of all let me say that basic styles like bold, italic, underline etc. were designed to be used with inline elements (elements like strong, span or b). This is written in API indirectly "The style definition that applies the bold style to the text".

Why whole line is being changed?
This is matter of approach we have decied to. When you click inside line of text and select h1 or div from format dropdown, the whole line is changed. If bold style was only applied to selection then user would end up with three lines e.g. paragraph with header inside would be changed to <p>first</p><h1>second</h1><p>third</p>. Browser would show it as three separate lines. We have decided that it is better to change whole line than to have 3 separate.

About your case - whether result would be 3 separate div or two nested divs <div>test before<div style="font-weight:bold;">test inside</div>test after</div>, in boh cases browser will show this as 3 separate lines of text. I don't think this is the result you want to have.

Please use inline tags for Bold and any other inline styles.

[sarahjones]

Thank you for your quick reply and explanation.

I now understand that your goal here was to support only inline text. We have built a plugin that inserts merge fields that can be (HTML) block elements, so you can see why this would be a problem for us.

Re: "in boh cases browser will show this as 3 separate lines of text. I don't think this is the result you want to have." Why not let your user decide? You have already given me the option to put attributes, and thus classes, on the element. This means I could make it behave however I want; I can make it an inline, inline-block, or block element depending on how I want it to look.

Please reconsider this bug report or consider adding this as a feature.

Thank you, ~Sarah

[Jakub Ś]

I want; I can make it an inline, inline-block, or block element depending on how I want it to look.

If you want to use div and then give to it display:inline; so that it behaves like span element then appropriate approach is simply using spans in this case.

Re: "in boh cases browser will show this as 3 separate lines of text. I don't think this is the result you want to have." Why not let your user decide?
Please reconsider this bug report or consider adding this as a feature.

We needed to come to and agreement and select in our opinion best approach (both can be considered confusing). We have decided that changing whole line instead of making three separate ones is less confusing to users and now we stick to it.
I'm sorry but this behaviour will not be changed at least not in the near future.

[sarahjones]

We cannot use a span element because that would be invalid HTML. As I mentioned, we built a plugin for ckeditor that adds merge fields, and these merge fields can contain block elements. We have been using all inline tags (enterMode was p), but have to switch to using divs for enterMode and also change the Bold/italics/etc because using inline tags causes invalid HTML. The invalid HTML causes the whole HTML contents to be rendered in an undesirable way.

Clearly, I disagree that changing the whole line instead of making three separate ones is less confusing because I filed a bug report. This behavior is inconsistent with all valid configurations of coreStyles_bold. My personal opinion is that if you are using the coreStyles_bold configuration and changing the tag to div, you probably know what using a div means.

Thank you again for your quick response. If we were to make the changes ourselves would you accept a pull request?

[Piotrek Koszuliński]

It's possible that the only thing you need to change the style's type (​https://github.com/ckeditor/ckeditor-dev/blob/master/core/style.js#L168) from block to inline. So from the code perspective it may be a pretty simple change. However, to accept the change we would need to have a tests suite for it (and in the styles system that means a lot of tests) and of course documentation. That's much more work for you and for us too, because everything needs to be reviewed and we do that thoroughly (especially, when we touch so complex parts of code like the styles system). I would like therefore to feel that we need this change, but I don't. I understand that some users expect to be able to apply a block style to a part of text, but nesting block tags like you proposed:

<div>The is my b<div class="bold">olde</div>d text</div>

is something that we want to avoid very much. For me, the expected results of applying div.bold to:

<div>The is my b[olde]d text</div>

are:

<div class="bold">The is my bolded text</div>

or:

<div>The is my b</div>
<div class="bold">olde</div>
<div>d text</div>

We do it now in the first way. The second way will most likely require significant coding effort (and the styles system is not a pleasant place), hundreds of tests and many weeks.

Therefore if you want to adjust the behaviour to your expectations quickly, then I recommend to fork CKEditor and do the necessary (and possibly small) change. In this case you will be able to proceed fastest and since the change may be small, you will be able to easily maintain your fork (I mean merging changes from the official repo).