ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Multi-root editor: cannot focus root which has only block widget inside

Open FilipTokarski opened this issue 1 year ago • 7 comments

📝 Provide detailed reproduction steps (if any)

  1. Go to multi-root editor demo
  2. Remove all content from one of the roots and add only a table
  3. Focus other root and then try focusing the root with the table (but do not click on the table)

✔️ Expected result

Focus should stay in the root, similarly to a "standard" editor:

https://github.com/user-attachments/assets/e8a13c03-4516-4aab-b4a6-99bbb8c8324d

❌ Actual result

Focus jumps back to the other root, it's not possible to focus a root which has only block widget:

https://github.com/user-attachments/assets/45c1f190-1130-406b-a821-5f42f85c38a3

❓ Possible solution

If you have ideas, you can list them here. Otherwise, you can delete this section.

📃 Other details

  • Browser: …
  • OS: …
  • First affected CKEditor version: …
  • Installed CKEditor plugins: …

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

FilipTokarski avatar Jul 26 '24 11:07 FilipTokarski

It does not need to ba a table, it can be anything that selected got a toolbar/tooltip; for example an image:

https://github.com/user-attachments/assets/d59de37f-27ad-47ed-8a72-61c7643ab170

pszczesniak avatar Jul 26 '24 11:07 pszczesniak

@FilipTokarski / @pszczesniak I checked it on Firefox, on latest docs, and I'm not able to reproduce this issue. Can you still reproduce this?

Mati365 avatar Aug 08 '24 06:08 Mati365

@Mati365 yes i can (Chrome).

pszczesniak avatar Aug 08 '24 07:08 pszczesniak

On Firefox it works as expected.

pszczesniak avatar Aug 08 '24 07:08 pszczesniak

Summary

Clicking on element with contenteditable="false" containing only one element with contenteditable="true" does not trigger focus (and selection change) on such contenteditable="true" child. It's reproducible in this test.zip, click in white area bordered with purple border and compare results between Chrome and Firefox. In the proposed change we trigger focus by the hand on such element before handling selection in the editor.

Debug notes

  1. It's not recent regression. It's possible to reproduce it on 39 version.
  2. Increasing timeout in this function shows that clicking on editable does not focus nested block element:

https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-engine/src/view/observer/focusobserver.ts#L106-L110

obraz

so it's performing fallback to previously focused node

  1. It's reproducible on 1 editable. It's possible to focus table by clicking outside editable (while having it focused) 🤯
  2. Looks like Chrome treats focusing nested contenteditable differently than Firefox. Maybe it's causing the issue? Example: test.zip. Click on editable element and check where the editing cursor really is on chrome and on Firefox.
  3. Focus tracker assumes that selectionchange event is fired after focusing element (in our case, it's editable). Check the code. After a small amount of time, it's flushing interface and then fallbacks to previous selected element because there is no selection (or that selection is incorrect) in the focused area. It works on Firefox, however. It's directly related to test.zip.

Mati365 avatar Aug 08 '24 11:08 Mati365

I stumbled upon this same issue today. I prepared demo for rerepo and then found this issue... Anyway, I'll share it, maybe it could be helpful - https://stackblitz.com/edit/1haefu?file=main.js.

https://github.com/user-attachments/assets/575164a2-a75f-4135-bc86-d88175111a74

What' interesting here is the other part of this issue (however, I'm not 100% sure if it's the same cause) is how document selection updates when you focus and then deselect different roots.

AFAIU selecting the root (and then deselecting it) should result in document selection (editor.editing.view.document.selection) being placed inside this focused root. However, with block widget inside middle root something happens and selection gets back to middle root, as if it's gets stolen by it:

https://github.com/user-attachments/assets/48bdddea-00cf-498d-ade7-1ee12a9463d1

@Mati365 maybe it could be worth checking, but if it's something different I can extract to a separate issue.

f1ames avatar Aug 09 '24 14:08 f1ames

@f1ames That's the same issue. I suspect that Chrome is not firing the selection change event properly after focusing editable that contains block element. So we have focus but there is no selection, and the editor is trying to fall back to the last known focused element because editable does not contain any cursor. The funny thing is that forcing chrome to focus first block or contenteditable element works.

https://github.com/ckeditor/ckeditor5/pull/16892/files#diff-5dd5f41942a4096889e59688767ae58a1755591bf98f1e52c8c8e9baba0609d0R254-R282

I extracted the issue to bare minimum HTML here: https://stackblitz.com/edit/stackblitz-starters-lf3smh?file=index.html

So on Firefox it used to work ok:

https://github.com/user-attachments/assets/fe8456b9-b30d-4546-90da-59b8b34d64f8

but on Chrome:

https://github.com/user-attachments/assets/091fca86-9c20-47d4-8fb8-8c7b75abb1b6

Mati365 avatar Aug 09 '24 14:08 Mati365