[FF] Focusing in image caption using tab key messes up selection handling

[scofalik]

Steps to reproduce

1. Go to https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/examples/builds/classic-editor.html.
2. Create list with two items before an image.
3. Focus in first item.
4. Press Tab .
5. There will be an error in console: NS_ERROR_FAILURE.
6. Now focus in second item.
7. Press Tab.
8. Selection is moved to the caption instead of indenting item.

This is the simplest scenario, but you can observe some interesting things. After step 8, focus in any paragraph and use Bullet List feature. Instead of that paragraph, the first item in the list is changed to bullet list (or un-listed if it was bulleted list). Also the button in toolbar is "active" all the time, so I guess that selection in model is stuck in the list item. But why it did not changed after using mouse key or arrow keys?

[scofalik]

Also it is impossible to focus image afters step 4.

[scofalik]

The same also happens sometimes after you do some undoes, but very rarely and I haven't yet understood when.

[Reinmar]

Yep, I stumbled upon this too. Pretty serious because the editor is dead after that. I'm afraid, though, that this is Firefox's native issue. Could you research it @scofalik or @szymonkups and report to Mozilla if I'm right?

[scofalik]

I'll try and research this issue.

[scofalik]

In this scenario, after the Tab key is used, we (for some reason) want to set the DOM selection to the place where it was before pressing Tab. I don't know why yet, but such is view selection. I'd suspect that the view selection would be in caption, but it is what it is.

Basically, the error boils down to this: https://jsfiddle.net/b6ef60aw/ . After launching the Fiddle, put the selection inside nested editable (at "Xyz"). After 5 seconds browser will try to move the selection and error will be thrown to the console.

This works fine in Chrome but throws in Firefox. For some reason Firefox throws when selection is in nested editable and it is moved by code to the outer editable.

PS. Removing all ranges from selection does not help.

[Reinmar]

Could you check in ckeditor-dev for some comments about that? I think I stumbled upon this in the past. And this might already be reported to Mozilla (you can look for the issue or report one).

[scofalik]

I am digging more into this why view selection is not correct. It seems that problem lies here:

document.on( 'focus', () => {
	document.isFocused = true;

	// Unfortunately native `selectionchange` event is fired asynchronously.
	// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
	// overwrite new DOM selection with selection from the view.
	// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
	this._renderTimeoutId = setTimeout( () => document.render(), 0 );
} );

It looks like document.render() is fired before SelectionObserver has a chance to set stuff. When I changed timeout to bigger, it started to work, even 10 was enough. In #3968 I can read that selection observer should fire render. Maybe this is the way we should go?

[Reinmar]

It makes sense. Focus is often asynchronous and slow. In CKE4 in a similar place we had a timeout of 200ms IIRC. However, let's have the lowest stable value here. If 10 is ok, then let it be 10.

PS. I recommend testing on a machine which is slowed down by some other task. E.g. think – building documentation :D

[scofalik]

I want to propose a different solution. Pushed to branch t/1153:

ckeditor/ckeditor5-engine@2d70a8b

Instead of trying to guess and kick in at the right moment, let's force re-rendering after selectionchange. It looks like selectionchange is fired always when focus is fired, it doesn't matter that the selection is placed at the same position.

Please guys take a look at it and see if there is something dangerous in this solution.

/cc @szymonkups @pjasiun @Reinmar

PS. It seems it solve this and related issues.

[Reinmar]

Well, we've been talking about forcing rerender after every selection change :D Maybe it's time? https://github.com/ckeditor/ckeditor5-engine/issues/796

[scofalik]

My concern is mostly about scenario where selectionchange is not called. If there is any.