Prevent UI from reacting to the conversion in some situations

[ma2ciek]

This issue is a continuation of the https://github.com/ckeditor/ckeditor5-table/issues/168.

In short:

Editors initialize plugins, UI, and then the data controller in that order. It might happen, that the data controller initialization is replaced with a custom implementation, which uses the conversion API to parse the initial data.

During such conversion, the view and the model are still empty. Among others, the view document selection is unset. The conversion triggers a cascade of events

TypeError: Cannot read property 'parent' of null
    at findAncestor (utils.js:18)
    at Object.getTableWidgetAncestor [as getRelatedElement] (utils.js:64)
    at WidgetToolbarRepository._updateToolbarsVisibility (widgettoolbarrepository.js:152)
    at BalloonEditorUI.listenTo (widgettoolbarrepository.js:87)
    at BalloonEditorUI.fire (emittermixin.js:196)
    at BalloonEditorUI.update (editorui.js:92)
    at Document.EditorUI.listenTo (editorui.js:63)
    at Document.fire (emittermixin.js:196)
    at View.on (view.js:165)
    at View.fire (emittermixin.js:196)
    at View.change (view.js:376)
    at View.render (view.js:390)
    at Model.EditingController.listenTo (editingcontroller.js:83)
    at Model.fire (emittermixin.js:196)
    at Model._runPendingChanges (model.js:705)
    at Model.change (model.js:156)
    at DataController.toModel (datacontroller.js:352)
    at DataController.parse (datacontroller.js:334)

After latest changes in the ckeditor5-table and ckeditor5-widget, the WidgetToolbar invokes the visibility callbacks for the table's tableContent toolbar, which operates on the position (which is equal to null as the selection isn't set at this point.

Such quick fix as:

https://github.com/ckeditor/ckeditor5-engine/blob/fd2005d2f84db05242d689ce3a31208f287e04c5/src/view/view.js#L161-L166

this.on( 'render', () => {
	this._render();

	if ( this.document.selection.rangeCount !== 0 ) {
		// Informs that layout has changed after render.
		this.document.fire( 'layoutChanged' );
	}
} );

...works and doesn't break editor and existing tests. I'm not sure if it's perfect (but editor's UI shouldn't respond if the selection is empty).

PS: IMO, the conversion should be more pure, in that case, parsing invokes UI actions. So the fix could be even more generic, preventing any UI action during the parsing.

[ma2ciek]

Alternatively, there's a fix proposed by @oskarwrobel:

Maybe ui#update should start to listen to view#render -> viewDoc#layoutChanged after data#ready.

[pjasiun]

Actually, we may try to set the default value of _renderingDisabled to true (see https://github.com/ckeditor/ckeditor5-engine/blob/fd2005d2f84db05242d689ce3a31208f287e04c5/src/view/view.js#L138)

This way, there will rendering will be disabled until the first conversion https://github.com/ckeditor/ckeditor5-engine/blob/fd2005d2f84db05242d689ce3a31208f287e04c5/src/controller/editingcontroller.js#L82.

This was the idea to solve another problem we had last week, but apparently, we did not need it.

On the other hand, I agree that we could block editor.ui#update event. I think it should be blocked in the editor.ui.updated() method (https://github.com/ckeditor/ckeditor5-core/blob/6d635380e1e10b3cb97f6e2673e52cff7c2ba113/src/editor/editorui.js#L92) so even if someone calls this method manually the event will be postponed until data#ready.

[ma2ciek]

After some testing and F2F talk we came up with @pjasiun to the conclusion that the above fix won't solve the problem and that the problem is a little bit deeper - EditingController shouldn't refresh view if there're no changes in the model.

So the proposal is to gather information in the model about changes and then fire this information in the _afterChanges event, so the EditingController will be able to decide whether to render its view or not. UI listen to the view.render so preventing view.render() from being called in such situations will fix the above problem.

[pjasiun]

I reopened this ticket because of https://github.com/ckeditor/ckeditor5-engine/issues/1660.