A couple of problems with core plugin onBeforeInput

[danburzo]

While investigating IME bugs, I came across a few problems with onBeforeInput in the core plugin:

1. Is it assumed here that e.data will contain a single character?

slate/src/plugins/core.js

Lines 89 to 98 in 239d97f

	const char = Character.create({
	text: e.data,
	// When cursor is at start of a range of marks, without preceding text,
	// the native behavior is to insert inside the range of marks.
	marks: (
	(prevChar && prevChar.marks) ||
	(nextChar && nextChar.marks) ||
	[]
	)
	})

In the case of a composition event, onBeforeInput will receive the entire composed string in e.data just before calling the compositionend event, so instantiating a Character with a multiple-char string may break somewhere. (At fist impression, seems the list insertion works either way)

2. I understand this line should mimic what a native insertion would be?

slate/src/plugins/core.js

Line 100 in 239d97f

const chars = initialChars.insert(startOffset, char)

If that's the case, it should take the selection into consideration, since you may be actually replacing some characters in the process.

This is all I have for now in regards to inserting composed words, I'll keep investigating — is there any other place that assumes e.data is a single char but then ends up pushing a multi-character string into the state? That may be the source of some of the problems we currently have with IMEs.

Right now I reckon these are causing:

1. Unnecessarily using a non-native insertion when it could have been native, due to mismatch between computing "native" outcome vs. "synthetic" outcome
2. Noticing that when overwriting text with a composed word, the outcome of the "synthetic" action is not accurate (selection is out of sync in the meantime), but the "native" one is, allowing a native insertion might fix some bugs.

[danburzo]

A big chunk of problems with IME is caused by the iOS autocomplete fix, as correctly identified in these pull requests:

• Check platform before adjusting selection #881 — it proposes making that transform only on IOS.
• fix IME input will replace characters after cursor #885 — it proposes making that transform regardless of platform, but not when composing, since that introduces regressions in that case at least.

If I were to choose between the two, I'd go with the one that limits it to IOS*, since leaving it around may have other implications other than composing. I'm also looking to see if the IOS fix can be achieved in a way that doesn't cause regressions with composing.

(*Although the fix might not be iOS specific since it treats mismatches between the selections in a general manner)

Edit: It does look like treating all iOS input events as native (or at least the ones containing e.data.length > 1) fixes the problem, and would be a less opinionated way to handle the discrepancy.

[danburzo]

A tangential update on IMEs / iOS autocomplete fix.

• I ended up building this http://danburzo.github.io/input-methods to help debug the events triggered for various composition actions
• I started documenting input methods here https://github.com/danburzo/input-methods/tree/master/methods (currently some for macOS and iOS) which can ultimately serve as a IME smoke test.

wrt to the iOS completion, starting with Safari 10.1, we have access to Input Events Level 2, which includes an inputType property which helpfully lets us know if it's a simple insertText (selection does not change) insertReplacementText (selection changes and some text gets replaced); so theoretically we can patch very specifically for at least Safari 10.1+.

[davidbonnet]

Thanks for your insights @danburzo:

• Yes, before changing the content in onBeforeInput, the current selection must be considered.
• Yes, monitoring the sequence of input events is the right approach, IMHO.
• Although interesting to check, I don't think it is necessary to compare between sequences of React's SyntheticEvent and DOMEvents, since this would be a React issue, not a Slate one.

Context

• The Input Method Editor (IME) API enables standard keyboards to input characters that are not directly represented on the keyboard itself, which occurs when writing in Chinese, Japanese, or Korean.
• Lately, the IME API is used on Android to also input suggestions in other languages.
• If input suggestions are disabled, normal keyboard events are issued and Slate works fine.

Proposed approach

1. Monitor sequences of events (with tools like http://danburzo.github.io/input-methods or https://optimdata.github.io/slate/#/mobile as shown in the screenshot below).
2. Define input scenarios that would generate various sequences of events: typing a word, typing a word and choosing a suggestion, deleting characters, deleting a range of characters when long-pressing, etc…
3. For each scenario, record the SyntheticEvent sequences and the resulting HTML on different OS versions (Android 4.0, 4.1, 4.4, 5.0, 6.0, 7.0, 8.0, and iOS).
4. Write tests that execute these event sequences an compare the output to the expected one. In particular, tests such as the ones for the core plugin could be used.

The source code of the event watcher is available in this fork.

[ianstormtaylor]

Hey @danburzo and @davidbonnet thanks for investigating this!

In light of the isNative concept no longer being part of Slate, are some of these IME-related issues now fixed?

[danburzo]

Hi, sorry for the hiatus — I'm back on the problem. I noticed the recent merge that removed isNative, I'd need to get back to speed with the problem and see how it impacts (in my previous tests, disabling isNative did not have a major impact on IME problems unfortunately).

[davidbonnet]

IMHO, this issue will not be tackled as long as there are no tests to make sure IME is well handled.

And, yes, it still fails on a Samsung Galaxy on Android 4. The example below shows the input of "Hello there" with the predictive mode enabled:

Note that disabling the predictive input removes the issue.

[abobwhite]

@ianstormtaylor @danburzo

So where are we regarding the IME issues? I am seeing issues #1114, #810, #854, #880, and #885. It seems like several have offered a solution but there has been some hesitance due to lack of familiarity to merge any. Can we use this issue as a place for continued discussion and resolution? This is a major show stopper for me that I hadn't realized before. I'm happy to help in any implementation/PR.

[davidbonnet]

I proposed an approach which consists of setting up tests to ensure that the editor works as expected. Lack of tests makes it very difficult to approve suggested solutions. I'm open to collaborate on this approach or alternative ones.

[ianstormtaylor]

Hey everyone, @danburzo thanks so much for opening this! I think some of these issues have been solved in newer versions of Slate. I'm going to close this one since it's got a lot of stuff going on, but feel free to open up individual issues for anything that remains!

[Nantris]

Holy crap @davidbonnet I just saw this link you posted: https://optimdata.github.io/slate/#/mobile

I know it's not working, but it's a big step forward. Can you explain the relevance of the gold box?

Thanks very much for your work on this issue!

[davidbonnet]

@slapbox: The golden box is a non-slate editable content element in order to record the unhindered input event flow.

[Nantris]

Thanks for the clarification. I suspected, but wasn't sure that was the only point.