Toggle attribute command is on when selecting an object with bolded text in a nested editable

[Reinmar]

That's, most likely, due to the command scanning text inside of the image. Since the image is an object, it should not go inside it.

[Reinmar]

The second issue is that the bold, italic and link buttons should not be even enabled when the entire image is selected.

In CKEditor 4 e.g. bold is enabled and applies itself automatically to nested editables. In CKEditor 5 the same behaviour is implemented currently. The question is whether this is good.

IMO, this is a bit confusing. I reported the issue because I haven't expected that bold can be applied to the entire image. Especially that the repercussion of enabling a button in such case is that it indeed needs to go "on" when first text node inside the selected element has the style applied. Note, that the bold button would not be on if the image was structured like this:

<image>
  Some text
  <caption><bold>Text</bold></caption>
</image

In CKE4 we've been discussing how a command could check its on/off state on objects like widgets but we haven't found some a good UX pattern for this.

[oleq]

Yeah, it's a little bit confusing. TBH, I wouldn't expect to see bold enabled on editor load when the first item in the editable is a captioned image (or later on, if the selected that object during editing).

OTOH, which is quite an off-topic here but I've spent some time working on it already and I'm eager to share my opinion anyway, if the editable was empty on editor load, I'd like to see buttons like bold, link or insert image available before the editor gets focus because users may actually apply formatting beforehand or start with an image.

[Reinmar]

if the editable was empty on editor load, I'd like to see buttons like bold, link or insert image available before the editor gets focus because users may actually apply formatting beforehand or start with an image.

Yes, that's what would happen. If the editor is empty then ToggleAttributeCommand and similar commands should be enabled. This must be implemented in their _refreshState() logic. That's also why we need #353. Otherwise, if there's no focus, applying e.g. bold would have no action.

[Reinmar]

This ticket needs to be moved to ckeditor5-basic-styles.

[pomek]

I investigated to this bug and determined that Schema.checkAttributeInSelection() enabled the bold/italic buttons because one of nodes (inside the image) accepts the bold/italic command execution.

As you said guys above, we need to disable checking the attributes if the selection contains an object. I guess it should be done in Schema.checkAttributeInSelection() method. Am I right?

I figured out another strange bug.

1. Select a paragraph, an image (with caption) and another paragraph,
2. Execute the bold command,
3. Click on the image,
4. The bold button is marked as active (it shouldn't) and disabled.

[Reinmar]

As you said guys above, we need to disable checking the attributes if the selection contains an object. I guess it should be done in Schema.checkAttributeInSelection() method. Am I right?

No, what you checked here is that the command is enabled (meaning – it can be executed) if there's some place inside a widget where it can be applied. This is fine.

What is wrong is that the command's value is true meaning that selection.hasAttribute() returned true and it shouldn't. The value should be false. So this is a different algorithm. Look how selection's attributes are refreshed (LiveSelection#_updateAttributes()).

You can test it on http://ckeditor.com/. If you select an image which has bolded text in its caption the bold button is off. But you can also use that button to apply bold inside the caption (if it wasn't there yet). We want exactly the same behaviour in CKE5.

[Reinmar]

I figured out another strange bug.

This is what I got:

And the only thing which is wrong here is that the bold command's value is true while it should be false.

[pomek]

Thanks for the reply. Now I know where I should look.

You can test it on http://ckeditor.com/. If you select an image which has bolded text in its caption the bold button is off. But you can also use that button to apply bold inside the caption (if it wasn't there yet).

Does it work as you said? After selecting the image, bold button is enabled but does nothing with the image.

[Reinmar]

Does it work as you said? After selecting the image, bold button is enabled but does nothing with the image.

Exactly. That's because the command is off (value==false). Mind the ending of my comment:

But you can also use that button to apply bold inside the caption (if it wasn't there yet).

[pomek]

Does it work as it should now?

[Reinmar]

Yes, this may be a little counterintuitive but this is how it should work for now.

I've had some doubts regarding the expected behaviour here because it's a bit less useful. However, the current behaviour, when the attribute command interacts with the widget is also confusing and that's why this ticket was reported originally (it was someone's feedback). When you click an image you think about the image. So the bold button going "on" in this case is confusing. Therefore, it should not.

The separate question is whether the command should be enabled at all. Perhaps styling commands should not be "on" for objects unless they can be applied to the object itself. Hard to tell.

Right now we're aligning this behaviour to that of CKEditor 4 and we can think about this in the future.

[pomek]

This bug is related to LiveSelection. I moved this ticket to https://github.com/ckeditor/ckeditor5-engine/issues/986 repository.