[CKEditor 5] Words inside a nested editable can't be corrected via `Proofread in dialog`.

[Mgsy]

Description

If you insert a word with typo to an image caption/table cell, then open Proofread in dialog, it won't display properly and it won't be possible to correct the word.

[jshaptala]

This behavior for Proofread in dialog is expected as the parent element of an image caption has contenteditable = false. In such cases we ignore everything inside non-editable elements, e.g. our parsing mechanism doesn't try to dig deeper if the parent is non-editable.

Unlike the Proofread in dialog mode, in the as you type mode we have more extended parsing options. Thus, in the same case with a nested editable/non-editable elements in the as you type mode the spelling/grammar problems will be detected and underlined. Actually, this behavior is reported in issue #4.

We understand that we need to implement a consistent approach for both cases (Proofread in dialog and as you type). Would you expect WProofreader catch problems in both cases even if the parent element is non-editable? Please suggest.

[Reinmar]

The structure of an image widget looks like this:

<figure class=image contenteditable=false>
    <img>
    <figcaption contenteditable=true>...</figcaption>
</figure>

Naturally, the proofreader in a dialog should not spell check things inside contenteditable=false. But then, there's the <figcaption> which enables contenteditable=true again, so ideally I'd see the proofreader in a dialog scanning those parts too.

In fact, the cE=false > cE=true nesting can be repeated. You can have images inside table cells and then there will be multiple cE=false and cE=true elements in the path, but in the end, the lowermost one has cE=true and it takes the precedence.

BTW, I checked how proofreader in a dialog worked in the past on https://webspellchecker.com/wsc-proofreader/#proofreader-ckeditor5 and from what I can see it must've been using getData()/setData() because it doesn't integrate with the undo and because I can see this in its content:

If it kept communicating with the editor through setData()/getData() then cE=false/true would not be a problem because those attributes are not used in the data. In fact, in your demo it works with tables very well, so it should also work with images.

OTOH, integration via setData()/getData() would not work with collaborative editing, track changes, comments, etc. Simply because on every change you override the whole content.

Therefore, closer integration that you worked on will be better. However, I'm not sure that you chose the best path. I assume that this new integration reads directly from the CKEditor 5's editable element. In this case, you indeed have to deal with cE=true/false content, but in fact, with much more. E.g., I can see that you now display the table's drag handle:

Currently, we don't have many complicated widgets, but even simple ones like media embed create problems. For instance, this video is playable (and you may e.g. mistakenly leave the page if you click something on it):

There may be much more complex media objects and you shouldn't embed them like this because it will make the content of the proofreader very messy.

If you want to continue going in this direction I'd recommend writing a filter which removes/hides all content which is problematic. Or rather – whitelist safe things. This applies to many things which aren't normally editable in CKEditor 5 (so, which are placed inside cE=false containers), but it may also need to apply to things which would normally be editable but can also cause problems – e.g. links.

[Reinmar]

PS. I think there are better ways to integrate the dialog with CKE5. E.g. still using getData() (instead of reading directly from the editable) to get cleaner content but not using setData() to change the content of the editor. Instead, you could map words in the content output with getData() to the exact positions in CKEditor 5's model and replace them there directly, with the proper editor API.

[jshaptala]

1. It seems that you didn't provide access to CKEditor 5 instance through DOM element as it is described in our Docs (Step 3). Without this step innerHTML of the element will be used, but we do not plan to use this approach for CKEditor 5. Instead, if you added the missed step, you will see that we
   use setData()/getData() as you can see on our website page.

BTW, by the end of this week, we plan to introduce a new release which will be using your ckeditorInstance. With this the extra step mentioned above will be no longer needed.

<textarea name="content" id="editor">This is a sample text.</textarea>
  
<script>
    ClassicEditor.create(
        document.querySelector('#editor')
    ).then((editor) => {
        editor.ui.getEditableElement('main').editor = editor;
    });
</script> 

@Mgsy and @Reinmar, please confirm that after adding the mentioned step the issue is no longer reproducible.

2. As to this:

OTOH, integration via setData()/getData() would not work with collaborative editing, track changes, comments, etc. Simply because on every change you override the whole content.

Yes, we do agree that using the Proofread in dialog mode with collaborative editing may cause a lot of problems and confusion. That is why we have already suggested in Slack to disable the Proofread in dialog mode at all. But as for now you prefer to keep it working as a nice to have feature.

In any case, we do have an API option for WProofreader that allows easily to disable the Proofread in dialog mode.

window.WEBSPELLCHECKER_CONFIG = {
    ....
    actionItems: ['addWord', 'ignoreAll', 'settings', 'toggle']
};

3. PS. I think there are better ways to integrate the dialog with CKE5. E.g. still using getData() (instead of reading directly from the editable) to get cleaner content but not using setData() to change the content of the editor. Instead, you could map words in the content output with getData() to the exact positions in CKEditor 5's model and replace them there directly, with the proper editor API.

Yes, we had such an idea to replace words in real time in both Proofread in dialog and the original editable element. But there are other cases to handle in this mode (e.g. user may exit the dialog without saving changes, etc.).

Nevertheless, the dialog mode is not the best choice to use in the collaborative editing as there will be always problems with the data synchronization between the data in the dialog and changed data in the original container.

We can work on the better options for easier navigation among problems in large documents without opening a static dialog as it is now.