[FF] Error after moving focus to the image caption using Tab

[Mgsy]

🐞 Is this a bug report or feature request? (choose one)

• Bug report

💻 Version of CKEditor

1.0.0-alpha2

📋 Steps to reproduce

1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
2. Put the caret at Heading 1.
3. Press Tab

✅ Expected result

Focus should move to the caption without error.

❎ Actual result

Focus moves to the caption and there is the error in the console.

📃 Other details that might be useful

Error

NS_ERROR_FAILURE: renderer.js:623
_updateDomSelection renderer.js:623
_updateSelection renderer.js:558
render renderer.js:235
render document.js:277
decorate/< observablemixin.js:327
fire emittermixin.js:273
decorate/this[methodName] observablemixin.js:331
FocusObserver/</this._renderTimeoutId<

GIF

Browser: Firefox 57 (Quantum).

Note: I'm able to reproduce this issue also in Firefox 56.

[ma2ciek]

I've checked it on Chome and Firefox@59 and listed below some results.

NS ERROR FAILURE it's a very generic Firefox error, it's hard to tell what actually goes wrong under the hood.

I see, that in the _domSelectionNeedsUpdate on FF, oldViewSelection is updated to the figcaption, but this.selection is not, so the Renderer tries later to move the selection to the place where the selection was before.

In Chrome, both this.selection and oldViewSelection were the same and pointing the Caption, so the Renderer didn't have to change it.

The _domSelectionNeedsUpdate is here:
https://github.com/ckeditor/ckeditor5-engine/blob/53ad06c8ea364a8ee940d5b2b8f9e74ee6d99a8b/src/view/renderer.js#L634-L654

I'll check whether this is not a problem with view selection.

It actually works with a try-catch block, so it should be just a case of not set Selection.

[ma2ciek]

Differences between Chrome and FF.

Note: the first rendering is because of the manually placed cursor at the beginning.

Chrome ViewSelection changes and rendering:

FF ViewSelection changes and rendering:

[ma2ciek]

I see that shift + tab from the caption doesn't work at all in the latest FF.

[ma2ciek]

It looks like the selection:change event is fired several milliseconds after the focus:change event on the FF and that implies wrong command execution order.

FF:

Chrome:

It looks like we can increase the timeout in the: https://github.com/ckeditor/ckeditor5-engine/blob/7755349c190f08e3806ae562f094823c5db57088/src/view/observer/focusobserver.js#L38 to solve that issue as we can't ensure correct order in a different way.

[ma2ciek]

Related: https://github.com/ckeditor/ckeditor5-engine/issues/795

[Reinmar]

Can't we make the code more bulletproof? Selection changes should not depend on focus changes and vice versa.

We could, of course, increase the timeout but it's hard to tell to what value. At some point the delay in which the editor reacts will be noticable but the editor might be still unstable in some specific cases on overloaded and slow devices.

[ma2ciek]

Can't we make the code more bulletproof? Selection changes should not depend on focus changes and vice versa.

Sure, I'll talk with @szymonkups about other possibilities. But I guess, it's not gonna be easy.

[ma2ciek]

After a F2F talks, we agreed that using longer setTimeout is ok for now as a quick bugfix, this will solve https://github.com/ckeditor/ckeditor5-engine/issues/1157 too. And then we should start working on the https://github.com/ckeditor/ckeditor5-engine/issues/796.