[FF] Error after performing undo in the image caption

[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 in Heading 1 and type something.
3. Put the caret inside the image caption.
4. Undo.

✅ Expected result

No error in the console.

❎ Actual result

There is the error in the console.

📃 Other details that might be useful

It could be related - #676.

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:333
fire emittermixin.js:269
decorate/this[methodName] observablemixin.js:337
EditingController/< editingcontroller.js:96
fire emittermixin.js:269
enqueueChanges document.js:243
execute undocommand.js:39
decorate/< observablemixin.js:333
fire emittermixin.js:269
decorate/this[methodName] observablemixin.js:337
execute commandcollection.js:67
execute editor.js:193
_addButton/</< undo.js:159
fire emittermixin.js:269
ButtonView/<.on.click< buttonview.js:230
callback template.js:947
fire emittermixin.js:269
domListener emittermixin.js:237

GIF

Other information
OS: Windows 10, MacOS X
Browser: Firefox

[Reinmar]

OMG... Perhaps FF is unable to move selection that quickly between nested editables. The first thing to do here will be to create a limited test case for it so we can report it because this is a recurring bug ;/

[Mgsy]

Here is another case:

Steps to reproduce

1. Open the console.
2. Put the caret in some paragraph.
3. Blur the editor by clicking on the console.
4. Focus the caption.

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:333
fire emittermixin.js:269
decorate/this[methodName] observablemixin.js:337
FocusObserver/</this._renderTimeoutId< focusobserver.js:39

[Reinmar]

Can we have a test case without the editor? @Mgsy, do you think you could try preparing it?

[Reinmar]

It turned out that creating a limited TC wasn't easy, so I checked whether there are ways to mitigate this problem. It turns out that there's a way to workaround this by focusing the root which is going to take the selection. So what I have now is:

_updateDomSelection( domRoot ) {
    ...

    domRoot.focus();
    domSelection.collapse( anchor.parent, anchor.offset );
    domSelection.extend( focus.parent, focus.offset );
}

And the problem doesn't occur anymore.

However, this isn't a complete solution yet. We should not focus the root if it doesn't have focus in the view. Otherwise, we may be stealing the focus from other places. OTOH, I'm not sure if we can call _updateDomSelection() at all in such cases. A TC to check would be:

1. Start Letters (2 users).
2. One user opens a link balloon on a certain piece of text and keeps focus in the URL input.
3. At the same time, the other user does something before the 1st user's selection which will need to result in updating the 1st users selection (e.g. splits the block).
4. The focus should not leave the URL input.

[Reinmar]

OK, _updateSelection() which calls _updateDomSelection() internally, does check the focus first. So perhaps adding that single line of code will be enough. I'll make a PR without automated tests and let's do some manual testing on it.