Rendering deeper changes fails to update text nodes

[Reinmar]

Example:

1. Starting view and DOM: <p><a>Text<b>X</b></a></p>.
2. Let's say that you type something at the end of this content which makes browser to create a lot of mutations and inserting the text in the wrong place (at the end of <p>) instead of at the end of <b> where we want to render it. It means that at this point we have the <p>'s children marked to be re-rendered.
3. Now, in the model everything is fine so we end up with such a view to be rendered: <p><a>Text<b>XY</b></a></p>.
4. What happens now is that the renderer tries to update first <a>'s children and then <p>'s children. It also wants to update "X" text node but since <a> will be rendered anyway, this is blocked.
5. _updateChildren( <a> ) is executed but it takes <b> through viewToDom() which then uses mapViewToDom() (because in the view that's still the same element) and we get the old... <b>X</b> DOM element.
6. This leaves us with an "X" instead of "XY".

This needs to be fixed in viewToDom() – it seems that it needs to somehow update the entire tree (not only take the element from the mapper) or mark its contents to be re-rendered by the renderer after it does its job.

Causes the following issues:

• https://github.com/ckeditor/ckeditor5-engine/issues/1093
• https://github.com/ckeditor/ckeditor5-typing/issues/120

[Reinmar]

I also encountered a simillar issue in ckeditor/ckeditor5-basic-styles#55 (comment). It's worth checking when we'll be working on this fix.

[scofalik]

There are two bugs connected with this and related tickets. Both are caused by the fact that DomConverter sometimes returns references (when elements are already bound) and sometimes create new elements. It's also confusing for the programmer, because it is hard to say what you can do with the results got from DomConverter. For example, you should not append returned elements to other elements - if those were already in DOM/View tree, you mess up the current structure. Another problem is that this indirectly mixes responsibilities of Renderer and DomConverter. If we go with the original idea to refresh nodes in viewToDom then why we need a Renderer in the first place -- the refreshed changes will be already applied in DOM.

[scofalik]

I've also pushed t/1125-wip which solves bugs described in:

• https://github.com/ckeditor/ckeditor5-engine/issues/1093
• https://github.com/ckeditor/ckeditor5-typing/issues/120

But I am not sure I like those solutions and I think that we need to discuss how DomConverter and Renderer should behave and how other parts of code should use DomConverter.

[f1ames]

The https://github.com/ckeditor/ckeditor5-typing/issues/120 issue is just solved by adjusting mutations observer as in WIP branch ckeditor/ckeditor5-engine@669ff4f#diff-57e194ad013563b5752afa147f447d67R209.

In this case the typed letter is placed outside of the link like:

<span class="ck-link_selected">
    <a href="https://ckeditor.com">
        Lin
        <strong>k</strong>
    </a>
    a
</span>

Which causes inconsistency with the model. And then trying to map the a text throws an error. In case of:

const newViewChildren = Array.from( domConverter.domChildrenToView( domElement, { withChildren: false } ) );

the a text child is not mapped as it was not fetched from domConverter.