Selection in Mutation

[pjasiun]

This one is tricky.

View and view events should build an abstraction laver over the DOM. In case of the mutation it means that old and new children are view nodes as well selection should be a view selection.

The problem is that new nodes are not yet in the view tree, they are only listed as new children of the parent element which is in the view tree. But that parent element does not have new children in the list of the child node (the change have to go to the model and back to be applied on the view document).

Now the selection. It usually land in that new node. It means that it need to convert DOM selection positions to the positions in the node which have no corresponding view elements in the view tree. It means that we can not use standard DomConverter method which assume that DOM and View are similar, or return null if it is not possible to return corresponding position. We need to find the way to put the position in the new view element which are only on the list of the mutations.

Fortunately we should not use this position very often. In most cases model will know better where to put selection after changes. On the other hand this information might be useful for instance for the typing: we may want to put the caret after inserted text on typing, but have selected word in case of autocorrection. Or if the autocorrection changed only one letter in the word the whole word should be selected or the caret should be after that word. To some extent, we are able to guess where the selection should be put based on the change which is done, but without the information where the native selection was we may have some issues.

[pjasiun]

There are more and more cases in which it will be tricky to predict where the selection should be put. The simplest case is: foo []bar and user press space so we have foo bar. It is not obvious which of these spaces is the result of the mutation and where selection should be put (foo [] bar or foo []bar).

[Reinmar]

Actually, when you type the second space one of them is usually turned into nbsp (by a browser ofc), and second is a normal space. That makes things even more complex :D.

Plus, of course, this is not only about spaces, but about all possible repeated characters.

[pjasiun]

This is why we should fix https://github.com/ckeditor/ckeditor5-engine/issues/379 too. nbsps should stay in the browser and you should have regular spaces on the View level.

[scofalik]

From what I take after toying a bit with MutationObserver, SelectionObserver, and Typing feature I see that we don't need to set up selection in Typing. It's set automatically after character is inserted, thanks to LivePositions.

The only problem with this approach is diff. Now, when there are multiple same characters in two diffed words, we don't know which character was inserted. This affects position on which new character is inserted, which in return affects how LivePosition in LiveSelection behaves.

Modifying diff smells a bit. Diff function should just show difference and should not be affected by outside parameters.

So it's a shame that we can't use tools that are already working, but I'd rather not modify diff function. Other than that, we could try to map DOM Selection available in mutation to view selection or try to deduce where the letter was inserted basing on DOM Selection.

I wonder which solution (of those three) is safest and cleanest, but it's probably best to go with DOM->View Selection mapping (if it's possible). If not that, we would need to check DOM Selection, whether it is collapsed or not and either pass it do diff or check if changes are before that position, or after, or inside selection range, etc.

[pjasiun]

Modifying diff smells a bit. Diff function should just show difference and should not be affected by outside parameters.

Modifying diff gives us nothing. You will never know if it was a first or a second letter inserted using only diff. You need DOM Selection to handle it.

[Reinmar]

Maybe prematurely, but I don't like the idea with not setting the selection. It forces us to base on the assumption that last change in the content is the right place for the caret. This is plainly wrong in cases like autocompletion or spellchecker where the middle letter of a word may be replaced by the browser while the selection should be (usually) after this word.

BTW, there's one related thing that came to my mind some time ago. Right now we use the sneaky snaky diff algorithm which gives us the minimal set of changes which need to be done to the content. For example, when we're replacing word "bafferr" with "buffer", according to this algorithm, we make two changes: replace a->u and delete r. However, what we really (semantically), do is replacing "bafferr" with "buffer".

Based on mutations we won't be able to understand the semantics correctly, but at least we could say that we replace "affer" with "uffe" (so – the change is between the first and last changed piece). Thanks to that the input feature would make one change in the model instead of two and this is a lot better when you look at this from the InputCommand perspective. Right now we need to call it multiple times per each small piece that has changed. So first we insert "u" over "a" and then... insert nothing over "r" (to delete it).

You can ask, why do we use the InputCommand if we want to delete "r". The reason is that the InputCommand keeps the state of the buffer. We can't use the DeleteCommand cause it will reset the buffer in the middle of the thing (and create separate undo step).

OTOH, we'll still have to use the InputCommand if someone replaces (using spell checker) "bufferr" with "buffer" – it's still more related to input, not deletion, so should land in this buffer. But I don't think that this is a problem that in specific situations the InputCommand will be used to delete pieces of content. We could perhaps consider adding "replace" command, but I wouldn't go that far.

PS. I thought about the InputCommand for a reason. It will have to set the selection after it changed the model. Otherwise, it would be useless (and we plan to use it for e.g. testing purposes). So, the typing feature will have to think about the selection anyway.

[scofalik]

Let me clarify.

Modifying diff gives us nothing. You will never know if it was a first or a second letter inserted using only diff. You need DOM Selection to handle it.

As I hinted, modifying diff would result in adding another parameter to the function. That parameter would probably be some kind of offsets connected to selection. Then, diff would have to use the passed value in a smart way to guess where the change really happened.

As I said, this stinks and I want to avoid such solution. We could, however, change/remove diff function completely (as @Reinmar proposed).

Maybe prematurely, but I don't like the idea with not setting the selection. It forces us to base on the assumption that last change in the content is the right place for the caret.

I am not sure what you are writing about and whether you understood the idea. I, too, want to avoid "leaving" selection after first changed placed. Actually, this is how it is coded right now. But, it's not like the selection is "left" there. It is set there.

So, right now, when you, say, right-click on word with typo and use browser spellchecker, the DOM Selection is set on the changed word. Then, if we don't mess up with it in Input feature, it stays there, at correct range. So, right now, not setting the selection give us correct results, except of situation when we insert multiple same letters.

I think that if we use mutation, it's perfect if we let the browser set the selection because we are using native browser algorithms. The thing is that we should not mess up the selection set by the browser. In order to do so, without changing diff function, we have to use DOM selection, "cache its parameters" and then use them to property restore it in the view.

So yeah, the idea is to set the selection, but basing on actual DOM selection.

BTW, there's one related thing that came to my mind some time ago. Right now we use the sneaky snaky diff algorithm which gives us the minimal set of changes which need to be done to the content. For example, when we're replacing word "bafferr" with "buffer", according to this algorithm, we make two changes: replace a->u and delete r. However, what we really (semantically), do is replacing "bafferr" with "buffer".

I also thought about changing how input works. I don't like that you make multiple one-letter changes. This also casued a bug that I already kinda reported here: https://github.com/ckeditor/ckeditor5-typing/issues/39#issuecomment-250725261. I don't see a reason not to replace the whole text instead of diffing.

Still, in this scenario, we need to use DOM selection to properly set view selection.

[Reinmar]

Still, in this scenario, we need to use DOM selection to properly set view selection.

Yep... Unless we'll have some brilliant idea.

E.g. the one I can think of (which is not necessarily brilliant) is converting a piece (current block or sth) of the DOM to a temporary view structure in which we'll calculate the correct position of the view selection, and then, translate it somehow to the document view.

[scofalik]

Of course, that's what I am talking about when I say that we need DOM Selection :).

[scofalik]

Okay so after F2F discussion here is what we are agreed on:

1. This issue is a more general and concerns handling selection in MutationObserver and passing ViewSelection or some kind of data together with viewMutations in mutations event. That data then would be used by features listening to mutations to set the correct selection. We don't have a precise idea how it would work but we believe that general solution would be difficult to implement. On the other side, we would base on DOM selection, which means that we don't have to think of special edge cases, cause browser would handle them.
2. Most of stuff discussed here is actually related to Input feature and typing repo. We can fix selection there. This is convenient because we want to introduce InputCommand and we'd like that command to handle selection. Also we anticipate beforeInput event which will change how we handle input so our solution from 1. could be obsolete.

We decided that we will fix what is to be fixed directly in Input feature. I'll make an issue in typing repo. https://github.com/ckeditor/ckeditor5-typing/issues/51

This issue will be left opened in case we want to implement general solution in MutationObserver. If you feel that this issue is not needed you are free to close it.

[scofalik]

I proposed some changes in ckeditor/ckeditor5-engine#626.

[Reinmar]

Fixed by ckeditor/ckeditor5-engine#626.