Can't remove block style after changing format

[jswiderski]

Type of report

Bug

Provide detailed reproduction steps (if any)

1. Clear editor contents and type a few characters.
2. Select what you have typed and apply "Italic Title" from Styles Dropdown.
3. Use the Format dropdown to change the formatting to normal Paragraph.

Old issue: https://dev.ckeditor.com/ticket/12526

Expected result

Plain paragraph of text should be produced - <p>test</p>

One style/format should reset other style.

Actual result

Paragraph with italic title is produced - <p style="font-style:italic">test</p>

The same thing is happening when you have a style without any styles defined (which maybe isn't super correct but it is possible in the editor) e.g.

{name: 'Heading1', element: 'h1',: {"styles": {"font-family": "Arial", "color": "red"}}},
{name: 'Normal Para', element: 'p'}

If you first apply 'Heading 1' and then 'Normal Para', you will end up with Red Paragraph having Arial font family assigned.

Other details

• Browser: Any
• OS: Any
• CKEditor version: 4.0+
• Installed CKEditor plugins: Styles, Format

[f1ames]

Upon switching to source mode and back, the leftover style is removed. This means ACF doesn't allow it and it should be in fact removed when using Format.

I think the proper approach could be checking if new element (upon using Format) allows previously applied styling and if so left it as is (as it also should be handled by Styles dropdown properly). If the style is not allowed in a new element, simply remove it.

[jswiderski]

@f1ames, please note that both tickets mention reset. If you apply it the way you have described (ACF wise) you will not change current behavior much.

Please also consider an example where you have e.g. Red H1 style, no Red P style but red font color is allowed for P by ACF. In described case, you will be left with Red P style which you shouldn't be able to create because it hasn't been defined anywhere (in any dropdown).

[jswiderski]

Info from original bug reporter:

In our scenario we would always want to remove the styles. Ideally very similar to how the "group" attribute for widget styles work - https://ckeditor.com/docs/ckeditor4/latest/features/styles.html#widget-styles

[jacekbogdanski]

@jswiderski could you tell more how you would see this features integration? It's quite misleading because we have both format and style plugins which seems to have the same purpose and probably could be configured with the same list of styles/formats. It causes me wondering how it should work in the first place between both plugins. Use cases:

Applying format to inline style

1. Select some text.
2. Set marker style
3. Set H2 format.

Actual <h2><span class="marker">text</span></h2>
Expected Seems to work fine, WDYT?

4. Repeat in reverse order (apply style then format).

Applying format to block style (and reverse).

1. Select some text.

2. Set Italic Title.

3. Select H1 format.

4. Repeat in reverse order (apply style then format).

Actual <h1 style="font-style: italic">text</h1>
Expected Seems to work incorrecty ie. when applying any block style or format against current block element, it should always replace block element with new format.

Current state of styles feature

I find out it a bit inconsistent that some of our styles like mentioned Italic Title set inline styles instead of semantic HTML. With semantic HTML, ACF won't remove styles when moving from Italic Title to a normal format. It seems still incorrect in the context of the issue ticket, but at least won't produce invalid editor content. Also, note that you can actually toggle styles and remove them using Remove Format plugin, so maybe current behavior (except ACF issue) should be treated as a feature, not a bug? It seems natural for me that the previous style is not removed but extended, however, I could miss the point of the whole feature as two standalone plugins that shouldn't cooperate.

[f1ames]

@jswiderski could you take a look on a previous #3649 (comment) ?

[jswiderski]

I find out it a bit inconsistent that some of our styles like mentioned Italic Title set inline styles instead of semantic HTML.

I don't mind the difference between inline and block styles because they behave differently. Inline styles can be copied to the next line on Enter key press and have the Remove Format button which allows erasing them. Block styles are one-line styles (you can't continue them on Enter) and have no Remove Format option thus I think it is should work in a way that when you change one block style to the other (or other format), the whole block (including styling) element should be replaced with the new one. For example if you have <h1 style="font-style: italic">text</h1> and want to change format to <p>, you should get <p>text</p>. Similarly if you have <h2><span class="marker">text</span></h2> and want to change the format to <p>, you should get <p><span class="marker">text</span></p>. All block styles should be removed but inline styles should be left alone.

Please also note this is not about ACF. In cases where it is disabled, you will be left out with styles like <h1 style="font-style: italic">text</h1> which you will have problems disabling because in order to do that, you will have to change back h1 to h2 and only then remove that style using styles dropdown. Actually this is the only way to disable given block style – toggle it in Styles Dropdown. If you apply italic title, you can only disable it by clicking it once again in styles dropdown. In my opinion, selecting different block styles from styles or format dropdown, should have the same effect as toggling and remove previous block style all together.

[Dumluregn]

@jswiderski we are working on the issue with @jacekbogdanski (there is already a PR: #4181) but we would like to ask you to confirm if we understand it correctly. Below I put a number of use cases with a proposed result, please take a look if everything is as you'd expect it. Also you can add more examples if you find them helpful or important.

// 1.

<h2 style="font-style:italic">
	<span class="marker">Hello world!</span>
</h2>

// format combo -> 'paragraph' => block style is removed, inline style is left:

<p>
	<span class="marker">Hello world!</span>
</p>

// 2.

<h2 style="font-style:italic">
	Hello world!
</h2>

// format combo -> 'h2' => style is removed, 'h2' format is reapplied:

<h2>
	Hello world!
</h2>

// 3.

<h2 style="font-style:italic">
	Hello world!
</h2>

// styles combo -> 'Italic title' => style is toggled:

<p>
	Hello world!
</p>

// 4.

<div style="background:#eeeeee; border:1px solid #cccccc; padding:5px 10px">
	<big>Hello world!</big>
</div>

// format combo -> 'paragraph' => block style is removed, inline tags are left:

<p>
	<big>Hello world!</big>
</p>

// 5.

<h2 class="red">
	Hello world!
</h2>

// format combo -> 'paragraph' => class is removed:

<p>
	Hello world!
</p>

// 6.

<h2 class="red">
	Hello world!
</h2>

// styles combo -> 'Italic title' => class is removed:

<h2 style="font-style:italic">
	Hello world!
</h2>

// 7.

<h2 custom-user-attribute>
	Hello world!
</h2>

// styles combo -> 'Italic title' => custom-user-attribute is removed:

<h2 style="font-style:italic">
	Hello world!
</h2>

[jacekbogdanski]

Closed in #4181