yjs / y-prosemirror

ProseMirror editor binding for Yjs
https://demos.yjs.dev/prosemirror/prosemirror.html
MIT License
335 stars 116 forks source link

Chinese / japanese input are syncd unexpectedly before compositionend #81

Closed jinzhubaofu closed 2 years ago

jinzhubaofu commented 2 years ago

Checklist

Describe the bug Chinese / japanese input will includes a composition process.

for example:

image

if I want to input "地" using pinyin,I need to press "di" first then press "blank". Before I press "blank", the "d" / "di" is useless and should not be synced.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'tiptap collaborative-editing demo'
  2. Open another tiptap collaborative-editing demo. Make sure your two browser tabs have the same room number.
  3. Get a chinese or japanese input on your pc / mac.
  4. input a chinese or japanese charator. for example “地”

Expected behavior “地” should not appear until you prese "blank", and "d" or "di" should not syncd.

Screenshots image

image

Environment Information

dmonad commented 2 years ago

Hi @jinzhubaofu,

we talked about this issue before, though I can't find the thread anymore. This feature would be very hard to implement. Also, I'm not sure whether this should maybe be reported to ProseMirror instead. y-prosemirror is simply syncing the prosemirror state. Filtering specific edits (e.g. characters that are created during a composition) would be very hard.

Is this an actual accessibility issue or this is .. more like a feature? To me, it doesn't seem too bad that the keystrokes are synced during composition.

Lastly, I want to say that I don't know at all whether we have access to the composition at all. To my knowledge, ProseMirror doesn't share that information with us.

jinzhubaofu commented 2 years ago

@dmonad Hi!

I think it's a bug because:

  1. all the intermediate inputs, "d" or "di", will be synced, and then be persisnted in a database. And then it'll be broken data.
  2. it's very confused for other collaboraters and hurts their experience.

I found this readonly property editor.view.composing in prosemirror's document. Is that usefull or enough?

Or maybe try to add compositionstart / compositionend handling in editorProps.handleDOMEvents? I'm sure composition events are working fine with contenteditable on Chrome or Safari. You can test it here

dmonad commented 2 years ago

all the intermediate inputs, "d" or "di", will be synced, and then be persisnted in a database. And then it'll be broken data.

What do you mean by "broken data"? Is it actually broken or just content that you didn't want to persist?

Is Google Docs also suffering from this behavior?

Or maybe try to add compositionstart / compositionend handling in editorProps.handleDOMEvents? I'm sure composition events are working fine with contenteditable on Chrome or Safari. You can test it here

ProseMirror still applies the changes to the ProseMirror-state while composing. And y-prosemirror is responsible for keeping the ProseMirror-state and Yjs in sync. We can't just filter some changes that we don't want to share: If y-prosemirror would simply ignore changes from Prosemirror while composing, y-prosemirror would overwrite the intermediate inputs when a remote user applies an update (because the Yjs state doesn't know about the intermediate changes that are not synced). This would be very frustrating as you'd not be able to compose while other users are editing. So the last option would be to disable syncing in the websocket provider while a user is composing. You'd not receive remote updates while composing and you would only sync after the composition ends. You could already implement this by calling provider.disconnect() while a user is composing and provider.connect() once the composition ends. This might also provide a bad user experience as composing clients would simply be unsynced.

If you are right, it might be better for ProseMirror to ignore composition changes and only apply the differences to the state when the composition ends. I don't know how we could solve this problem in y-prosemirror.

jinzhubaofu commented 2 years ago

@dmonad Ok, I got it, you are right.

It's prosemirror's decision whether update the editor's state within composition or not.


PS: Google Doc does sync the "broken data", and so does notion. But some other collaborative docs do not, such as Lark Docs. Maybe it's created by a Chinese company and most users have to use input method all the day.

zwz commented 1 year ago

@dmonad IMHO, the bad thing for yjs is that the persisted data grows fast for CJK documents, since "di" (which I guess is recorded in deleteset) is stored as well as "地". And it gets even worse in the prosemirror-versions demo, as many snapshots would be persisted.

Recently I came across this post https://discuss.prosemirror.net/t/programmatically-start-compositionevent-with-prosemirror/4454/6. I don't know if it helps for this issue, as Marijin says that you could pause sending of steps when a composition is active. I am wondering if it is possible to support pause/resume the yjs sync process (by like setMeta('composing', true/false)).