ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

[Safari] Reverse typing effect after focus change, e.g., outdent button causing text to insert backwards.

Open david-merritt opened this issue 2 years ago β€’ 15 comments

πŸ“ Instructions

  1. Open Safari
  2. Navigate to: https://ckeditor.com/ckeditor-5/demo/feature-rich/
  3. Remove all text from CKEditor input
  4. Click into the CKEditor input
  5. Click the Outdent button
  6. Start typing

βœ”οΈ Expected result

Text should input normally

❌ Actual result

The text is input in reverse

πŸ“ƒ Other details

  • Browser: Safari
  • OS: macOS
  • First affected CKEditor version: Latest

If you'd like to see this fixed sooner, add a πŸ‘ reaction to this post.

david-merritt avatar Aug 01 '23 06:08 david-merritt

I cannot reproduce it, could you share a video?

https://github.com/ckeditor/ckeditor5/assets/9881379/e71164d5-5385-4672-bb47-16701fb84b45

Witoso avatar Aug 01 '23 11:08 Witoso

@Witoso I believe this may be another case of "backwards" typing. We've also seen this type of behavior in Mac where no performance issue needs to be present for it to reproduce. This may be one of the cases I called out in the bug: https://github.com/ckeditor/ckeditor5/issues/14569#issuecomment-1650106287 where I believed there could be additional cases in this platform (Mac).

Internally, we've had a number of users claim similar things where performance issue has not been present (again, Mac!) but I've yet to actually reproduce it myself.

From my general understanding, the CMD+TAB action back into the editable will actually begin to insert text backwards (sticky cursor) -- however, based on my testing with @niegowski 's change, I was unable to reproduce it further. I personally don't use Mac often so it may just be me not being thorough enough.

Could there be another area where things like indent/outdent could affect selection not being updated?

urbanspr1nter avatar Aug 01 '23 11:08 urbanspr1nter

The demos on the page are not updated yet, so there's a chance it will help with this case. @niegowski WDYT about the indent?

Witoso avatar Aug 01 '23 11:08 Witoso

This case is different. While clicking on the disabled button (in Safari) it gets focused but the selection is still in the editable:

2023-08-01 15 22 01

But since editable is not focused, we do not touch the DOM selection so the selection is not updated.

niegowski avatar Aug 01 '23 13:08 niegowski

Oh, it's about the outdent :facepalm:

Witoso avatar Aug 01 '23 13:08 Witoso

This case is different. While clicking on the disabled button (in Safari) it gets focused but the selection is still in the editable:

2023-08-01 15 22 01

But since editable is not focused, we do not touch the DOM selection so the selection is not updated.

@niegowski Yeah this is something that I get confused about. πŸ˜– This happens on other scenarios too, but the basic trigger is the same. The text box generally looks focused at the moment the action is taken, but it isn't? This seems to be a Mac-only thing...

urbanspr1nter avatar Aug 01 '23 13:08 urbanspr1nter

Oh, it's about the outdent 🀦

It's about any disabled button in the toolbar.

If the button is disabled we probably should not focus the button on click (but still be focusable by the keyboard). But I'm not sure how it conforms to accessibility rules. cc @Comandeer?

niegowski avatar Aug 01 '23 15:08 niegowski

The focusability of disabled controls is about the discoverability of these controls and it's mainly mentioned in the context of a keyboard focus. I would argue that in the case of a mouse user, the discoverability is better achieved in other ways (e.g. hovering the disabled button). Due to that, I'd say that it's safe to disable the focus on clicking and keep enabled only the keyboard one. We've also done it in CKEditor 4 and it worked well.

Comandeer avatar Aug 02 '23 15:08 Comandeer

Another case #14830 of a similar behavior.

I wonder @niegowski what could be our actions here:

  1. Don't focus disabled (workaround for the browser's issue)
  2. Could we report somewhere upstream to Safari? WDYT @Reinmar?
  3. Should we "defocus" editable in Safari (is it even possible?) when focus lands outside the editable?

Witoso avatar Aug 21 '23 12:08 Witoso

I would like to suggest taking a look at #14830 because I provided a test case that allows one to reproduce the issue using a button element outside that exists outside of the editor’s realm. The disabled toolbar button is just another case of this and (at least there) you could possibly call event.preventDefault() to avoid the focus shift.

In the issue I have explained that document.activeElement in that situation points to an element somewhere else on the page (including the document.body). Maybe you could suppress typing events while the editor is not focused?

dtdesign avatar Aug 21 '23 12:08 dtdesign

Another case #14830 of a similar behavior.

I wonder @niegowski what could be our actions here:

  1. Don't focus disabled (workaround for the browser's issue)
  2. Could we report somewhere upstream to Safari? WDYT @Reinmar?
  3. Should we "defocus" editable in Safari (is it even possible?) when focus lands outside the editable?

I've run into similar issues like this, and have found that at times focus is ripped away improperly on Mac due to some forced blur event. We did something similar to (3).

This problem causes the DOM selection to "disappear". The editable is still in some weird focused state. How we were able to get around this temporarily was to register a beforeinput event at low priority, and on some insertion of the text, perform some operations to "refocus" by dispatching the focus again against the editable through manual DOM event.

Note, the issue doesn't seem to be limited to just these button clicks, it seems to be a general problem overall where unexpected focus stealing from the editable where it doesn't have a chance to (for lack of better term) clean up some state causes it to go into a reverse typing mode. (As @niegowski mentioned, this issue presented here is different from the other one that was fixed)

urbanspr1nter avatar Aug 21 '23 12:08 urbanspr1nter

Hi I see issue: https://github.com/ckeditor/ckeditor5/issues/14830 was closed. Does this mean that this issue has resolved as well? If so what release will it be deployed in?

david-merritt avatar Sep 25 '23 03:09 david-merritt

Hi, it was closed to not duplicate discussion on multiple issues. This is the main one for this problem, and the discussion about it.

Witoso avatar Sep 25 '23 07:09 Witoso

Just to add another way to reproduce this issue, I stumbled on this one recently.

While using Safari, in the CKEditor classic demo editor, if I click into the table menu and move my mouse around and then close the table menu, text starts being inserted in reverse order.

https://github.com/ckeditor/ckeditor5/assets/14916556/d532d242-cbd7-4861-b3be-a30dfb47ed99

slotterbackW avatar Nov 20 '23 19:11 slotterbackW

Another scenario for the sticky cursor in Safari:

Steps:

  1. Focus the editor.
  2. Click Upload image from computer.
  3. Click ESC or Cancel.

Results: Sticky cursor and typing in reverse.

https://github.com/ckeditor/ckeditor5/assets/50703222/0cd3410e-ecc6-42ac-8c38-134bce645a0b

Notes: This was working correctly in CKEditor v35.2.0, and started to be an issue in v36.0.0

macOS 14.0 Safari 17.0.0

michalbilik avatar Dec 07 '23 08:12 michalbilik

The work for this issue and others related is completed in the https://github.com/ckeditor/ckeditor5/pull/16289, and we finalized the round of internal (successful) tests. I encourage everyone to test the PR if you have a chance. It should be merged in the following days, and will be a part of the next release.

niegowski avatar Jul 01 '24 12:07 niegowski

Thank you!

david-merritt avatar Jul 03 '24 13:07 david-merritt