Ckeditor5: Selection observer should call render

[pjasiun]

The idea is that observers not only listen on DOM changes and fire view events but also take care about re-rendering DOM if it needs to be rendered. Especially, if the model did not fire change nor changesDone events it's observers role to bring DOM to the same state as view is, by calling render().

Note that it's renderer who should compare DOM and view and do nothing if there is no good reason to render anything. Observers should be free to call render whenever the feel that something may need the update.

render was removed from the SelectionObserver by this commit: https://github.com/ckeditor/ckeditor5-engine/commit/f4c0d25398bf9a85376e2f24f32c26cff188e16e

Because of it 'noselection' manual test does not work: http://localhost:8125/ckeditor5-engine/tests/view/manual/noselection.html

It's also connected with other issues: https://github.com/ckeditor/ckeditor5-engine/issues/795

[Reinmar]

I needed to change a code of http://localhost:8125/ckeditor5-engine/tests/view/manual/inline-filler.html a bit to make the test pass now.

Also, the http://localhost:8125/ckeditor5-engine/tests/view/manual/immutable.html test doesn't work now due to this.

[szymonkups]

I am working on UIElement rendering now. All DOM selection that lands inside UIElement should be converted by DOMConverter to selection before/after UIElement. This means that each selection change inside UIElement is converted to the same selection. When SelectionObserver is not calling render(), we end up with "dirty" selection in DOM having correct selection in view and model.

[ma2ciek]

In https://github.com/ckeditor/ckeditor5/issues/676 and https://github.com/ckeditor/ckeditor5-engine/issues/1157 we face a problem, that the focus and selectionchange events fire with some delay between them, which results in messing up the editor's selection (see https://github.com/ckeditor/ckeditor5/issues/676#issuecomment-349307800). We need to make the selection depend more on the selectionchange event, less on the focus event or make it somehow smarter.

Observers should be free to call render whenever the feel that something may need the update.

For now, calling render() between selectionchange and focus events results in errors.

[ma2ciek]

ckeditor/ckeditor5-engine#1153 and ckeditor/ckeditor5-engine#1155 are also connected with that topic.

[ma2ciek]

And, I've omitted one important thing, rendering in both FocusObserver and SelectionObserver won't fix the problem because sync rendering in the FocusObserver part renders selection in the wrong place (DOM selection at this stage is not updated and is taken from the wrong place) and SelectionObserver can't fix it later to the actual state.

Eventually, SelectionObserver might later try to fix it, but it doesn't look good. It would create a gap between two fired events where the selection is not at the place where it should be.

[Reinmar]

Yep, the only right place to render is after selection change (otherwise we may override the selection which ultimately breaks it). This makes sense as long as selection change is always fired together with focus so this needs to be verified. So far we've seen that focus always triggers selection change, but this is the kind of things you can never be sure. It may turn out that the solution will be to always render after selection change but also add some safety mechanism in the focus observer which would trigger rendering if there was no selection change in something like 500ms.