Autoformatting styles trigger within inline code

[oliverguenther]

🐞 Bug report

💻 Version of CKEditor

v11.0.1

Classic editor with at least enabled plugins:

• Code (Basic styles)
• Italic (Basic styles)
• Autoformatting

📋 Steps to reproduce

1. Create an inline code element
2. Type something like text with _underscores_.

✅ Expected result

Autoformatting is not expanded within code element

❎ Actual result

Autoformatting rules expand within code element

📃 Other details that might be useful

This happens with all styles within autoformatting since there's no construct of where this will be disallowed. I wonder if maybe the autoformatting plugin could export an attribute that we can set to elements that will result in skipping extraction?

(It will also happen in the block code plugin elements, for example)

---

Add 👍 reaction to this post if you'd like to see this issue fixed.

[Reinmar]

I've been thinking a bit about this and I first wanted to answer that it should be controlled by the schema. That these two attributes cannot exist at the same time. That'd disable the buttons as well as it should disable the autoformatting feature.

But, we've got a problem here: autoformat was implemented incorrectly and it doesn't use commands but it set attributes directly. I even commented in the linked ticket that: "I think that the autoformat should abort doing any changes to the content if the command is disabled."

One more thing that I'd like to check is whether disallowing an attribute based on another attribute works with the schema. I don't remember now what was the final decision about it.

[Reinmar]

One more thing that I'd like to check is whether disallowing an attribute based on another attribute works with the schema. I don't remember now what was the final decision about it.

This check should be able to disable italic in bold:

editor.model.schema.addAttributeCheck( ( context, attrName ) => {
	if ( inTextWithBold( context ) && attrName == 'italic' ) {
		return false;
	}
} );

function inTextWithBold( context ) {
	return context.endsWith( '$text' ) && context.last.getAttribute( 'bold' );
}

I made a demo in https://jsfiddle.net/7Lrhxp0b/6/. It works, but only for non-collapsed selections. Which means that we've got some bug with how AttributeCommand checks isEnabled for a collapsed selection.

[Reinmar]

But, we've got a problem here: autoformat was implemented incorrectly and it doesn't use commands but it set attributes directly.

Actually, autoformat does check the schema via:

const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey );

// Apply format.
formatCallback( writer, validRanges );

But it does not check whether it should remove the _ or * characters so that's the result on the demo I posted above:

[Reinmar]

There are some more discussions about implementing this rule in https://github.com/ckeditor/ckeditor5-basic-styles/issues/77. There are some more aspects that one would need to take into consideration. That's one thing.

The second thing is that we need to disable either those other inline styles in <code> by default. Or disable autoformatting only. I'm not sure what's the most commonly expected behaviour. Do you think that inline styles like bold or italic should in general be available in <code>?

[oliverguenther]

Please excuse my late reply, I've been on vacation for quite a while and only got to catch up open ends now.

Do you think that inline styles like bold or italic should in general be available in <code>

My personal opinion is that within code, no other formatting should be possible. But I do not have much to base this opinion on, so it may be useless in this discussion.

I wonder if the immediate issue could be addressed around by checking that an autoformatting character should only be triggered if it's not within a word boundary. I find it confusing that foo_bar_bla does trigger, when I expect that only foo _bar_ bla should. This of course does not address the question of validity.

[philippfreyer]

Side-note here. I stumbled upon this issue today. AutoFormat does not only run rogue here, if you type common variable and method names in Python (names with multiple underscores in them), then autoformat triggers sometimes (also sometimes it does not), if you keep typing, then it may also remove some characters that you typed before while you are typing new ones - even if you already finished a word like:
this_is_a_long_method_name_with_multiple_underscores Any text behind this may cause some characters of the word before to vanish while you are typing if autoformat triggered on the underscores before.

I reported this behavior first to OpenProject, which uses ckeditor as well and recorded a video regarding this weird behavior: http://sendvid.com/p4aubuv8
Posting it here just in case this is another issue with the same plugin.

[Reinmar]

There are two aspects to this issue:

• Having a way to disable the autoformat in certain cases. This could base on Plugin#forceDisabled() and is easy to add. External features will be then able to control this. This would allow turning autoformatting off in certain cases (like when in <code>).
• Additionally, autoformat should check the schema for whether it can apply a certain format in the current position. This is tricky as there are many corner cases. It also may not be a solution itself because someone may want to allow applying italic in code snippets if it's done manually (via the button) but disallow doing this via the autoformat.

[neongreen]

Do you think that inline styles like bold or italic should in general be available in <code>

I think they should ideally be available not only in inline code, but also in code blocks. This is useful for rich code snippets in tutorials — highlighting some part of the snippet for the reader, etc.

[Reinmar]

BTW, a quick workaround is to use the schema configuration to control what styles can be applied in a <code>:

// Disable italic in code:

editor.model.schema.addAttributeCheck( ( context, attrName ) => {
	if ( inTextWithCode( context ) && attrName == 'italic' ) {
		return false;
	}
} );
function inTextWithCode( context ) {
	return context.endsWith( '$text' ) && context.last.getAttribute( 'code' );
}

This works fine (it also seems that we fixed the issues that I described in #1239 (comment). However, it makes italic completely disallowed in <code> so you can't apply it manually (via the toolbar button too).

Therefore, the first point from #1239 (comment) would still help a lot. 

I'm adding this ticket to the current iteration with this scope:

1. Implement Autoformatting#isEnabled check in block and inline autoformatters.
2. Preconfigure the Code feature to disable Autoformatting(by using forceDisabled()) when in selection is in preformatted text.
3. Document this in the autoformatting feature guide in a section "Disabling autoformatting in certain elements".

[rinchen-shortcut]

I'm hoping to see this fixed soon.

[tomalec]

I've prepared a change that covers 1 & 2 from #1239 (comment)

1. Implement Autoformatting#isEnabled check in block and inline autoformatters.
2. Preconfigure the Code feature to disable Autoformatting(by using forceDisabled()) when in selection is in preformatted text.

But, I'm not sure if that's really enough, consider following scenario:

1. Open an editor with <p>Text <code>code_here;</code> more text.</p>,
2. add _italic_ at the very end.

I'd expect it to result with

<p>Text <code>code_here;</code> more text.<i>italic</i></p>

, but without changing the RegExps that matches the ` (backtick), it results with:

<p>Text <code>code<i>here;</i></code><i> more text.</i>italic_</p>

[tomalec]

The behavior shown above is a result of Autoformatting taking the text from the entire line into consideration.

At the F2F meeting with @oleq and @niegowski, we decided that the autoformatting feature, should not affect <code> elements before the selection. But we still would like to support other styles intersection, like: Text in _italic and **bo_ld** - > "Text in italic and bold"

(Note: neither Github, Slack nor Notion support this)

So, we would add another item to the scope:
4. Autoformat should consider only the text after the last code attribute.

(Another rejected, but more breaking alternative was to make autoformat operate only on currently focused TextNode and make intersecting styling unavailable through autoformat. It would be still available with manual styling, also nested styling would be available.)

[oleq]

Let's make sure the feature does not collide with the code attribute first. In the follow-up, we can improve (or get rid of) the intersecting auto-formatting.

[tomalec]

Followup issues:

• Do not autoformat intersecting styles #7364 - on intersecting styles
• Refactor *AutoformatEditing.js #7366 - code cleanup
• Do not auto format after ` (backtick) #7368 - `foo_bar_` -> foo_bar_/ foobar