Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

[IME][Safari] Fix IME composistion broken Issue in empty paragraph in Safari #217

Closed
wants to merge 1 commit into from

Conversation

farthinker
Copy link

This issue was originally described in ckeditor/ckeditor5#1333.

After some digging, I found the _updateSelection call in render method unnecessarily update the DOM selection after I start composing in an empty paragraph, which breaks the composition.
I continued looking into the _updateSelection method. DOM selection is different from view selection when we type the first character, which is why _domSelectionNeedsUpdate returns true.

So why the DOM selection is different from view selection only after we type the first character in an empty paragraph? And only in Safari? I debugged related code with several console logs, and then found out a weird behavior of composition in Safari: the selection won't be collapsed while composing. More precisely, when we type character c with Chinese IME in an empty paragraph, the selection would be [c], instead of c[]. You can have try with my test code in Safari: https://codepen.io/farthinker/pen/KOoNJY?editors=1111.

Thanks to this weird behavior in Safari, _handleTextNodeInsertion method cannot correctly set resultRange of input command, and input command only predict resultRange from the length of inserted text and default it to be collapsed.

BTW, _handleTextMutation method would predict the resultRange from the actual DOM selection, so composition won't break in the following typings.

I think it's reasonable to always respect the actual DOM selection while composing, no matter in what browser, as any unnecessary selection/DOM update could break the composition. But the problem is when we handle text node insertion we cannot convert actual DOM selection to view selection, because the text node is already in the DOM, but it's not been created in the model yet. All we can do is try our best to guess. My fix solution is also a prediction about the result selection, but in a more precise way.

@farthinker farthinker closed this Aug 8, 2019
@farthinker farthinker deleted the t/1333 branch August 8, 2019 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant