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

[farthinker]

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]

CI build fails, any suggestions? BTW, tests are all green in my local machine.

[jodator]

@farthinker sometimes it might happen - some git reference error. I've restarted the CI job to dobule-check.

[jodator]

OK - I think that your CI Job wasn't tested with external PRs (coming from other repositories). So this PR should be tested locally before merging.

[coveralls]

Coverage remained the same at 100.0% when pulling 5a6ee67 on mycolorway:t/ckeditor5/1333 into 7f90ace on ckeditor:master.

[jodator]

@farthinker & @Reinmar - the CI Job was fixed by ckeditor/ckeditor5-dev#543. So we're good to properly handle this PR.

[jodator]

Hi @farthinker - I've been looking to your PR and I have some troubles in verifying it:

AFAICS while in composition Safari tries to re-set the contents of the range. It looks like we're changing it then and the whole typed text is re-inserted at the current cursor position. This produces the duplication as long as user types in composition mode.

As you wrote - the best way would be to not touch the DOM selection when in composing mode. Or render the selection as the IME supports that - i.e. setting a non-collapsed selection.

A similar thing happens on master.

ps.: It's sad that it only happens on Safari while Chrome works perfectly.

[jodator]

@Mgsy could you double-check this?

[Mgsy]

Unfortunately, I'm also unable to verify this fix.

[farthinker]

@jodator @Mgsy Thanks for your review. I've reproduced the issue with another IME. Could you plz provide the info about your os version, safari version and IME. I'll look into it later.

[Mgsy]

@farthinker, I've tested it on macOS High Sierra 10.13.6, Safari 12.1.2 and with Pinyin - Simplified language.

[farthinker]

@jodator @Mgsy some IME cannot bear any DOM selection update while composing, even if the new selection is identical with the previous one. I could not find a way to fix it in the typing package, so I try to fix the issue in the render process. Plz take a look at https://github.com/ckeditor/ckeditor5-engine/issues/1782 and ckeditor/ckeditor5-engine#1783.