Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated characters in Safari with IME - unreliable cause and solution #944

Closed
quirm opened this issue Jun 27, 2019 · 12 comments

Comments

@quirm
Copy link

commented Jun 27, 2019

Issue details

There is a problem that editor duplicates text characters in Safari, when marks are applied and IME input is active.

image

ProseMirror versions

"prosemirror-commands": "^1.0.8",
"prosemirror-dev-tools": "^2.1.1",
"prosemirror-dropcursor": "^1.1.1",
"prosemirror-history": "^1.0.4",
"prosemirror-inputrules": "^1.0.4",
"prosemirror-keymap": "^1.0.1",
"prosemirror-model": "^1.7.1",
"prosemirror-state": "^1.2.3",
"prosemirror-tables": "^0.8.1",
"prosemirror-transform": "^1.1.3",
"prosemirror-view": "^1.9.10",

Affected platforms

  • Safari only as far as I know

Detailed description

The root cause is not 100% clear to me because I could not reliably reproduce it but I suspect it has something to do with native selection. Please bear with me, this description is a bit longer.

I have two simple test cases:

  1. type "n" with Pinyin IME keyboard; capture the log
  2. type "n" with Pinyin IME keyboard while mark "bold" is active; capture the log

These two actions are identical, while the second one is causing troubles. I swizzled few methods to find out how stack traces differ under the hood:

image

In Safari, once a selection is re-selected right after compositionstart, compositionend will not happen anymore and any keypress with an active IME input will be calling redundant compositionstart, followed by compositionupdate. Event beforeinput also will not match correct types in the right order.

I pin pointed the issue in setSelection function:
https://github.com/ProseMirror/prosemirror-view/blob/master/src/viewdesc.js#L331

There is a blocking part which should prevent re-selection from happening:
https://github.com/ProseMirror/prosemirror-view/blob/master/src/viewdesc.js#L343-L346

I logged the state after the if-clause from the link above (it can be done through a breakpoint):

console.log(anchorDOM.node, anchorDOM.offset, domSel.anchorNode, domSel.anchorOffset);
console.log(headDOM.node, headDOM.offset, domSel.focusNode, domSel.focusOffset);
console.log(headDOM.node.textContent);

This is the output:

image

The interesting part is, that an element with the cursor (anchorDom or headDom in this case) will show nn as its content but node.textContent is only n. Function isEquivalentPosition will fail in this case to prevent re-selection and then the whole IME input lifecycle breaks down. I could not figure out why and I could reproduce it only a few times with a single <div contentEditable=true /> element. My guess is, that something under the hood returns a differently evaluated content in Safari DOM.

How to fix this?

Blocking setSelection call was the only reliable fix that worked:
https://github.com/ProseMirror/prosemirror-view/blob/master/src/selection.js#L57

I changed it to following as you can't really do too many things or even select text with an active IME input:

if (!view.composing) {
  view.docView.setSelection(anchor, head, view.root, force);
}

@marijnh, does the change make sense and could be used as a fix? I can create a PR if you are not against it.

@marijnh

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

Blanket-blocking setSelection when IME is active will also interfere with, for example, inentionally moving the selection away when composing. I'll dig up my macbook and try to look into this soon.

@quirm

This comment has been minimized.

Copy link
Author

commented Jul 3, 2019

I've just tested it again that this fix will cause a reversed problem when no marks are present. For example, typing ni hao will start duplicating characters after a space char. No native selection change is involved in this case :/

image

@marijnh

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

This patch might help with this. I've released it as prosemirror-view@1.9.14-prerelease — it'd be great if you could test it and report whether it solves this issue.

@quirm

This comment has been minimized.

Copy link
Author

commented Jul 29, 2019

I tested 1.9.14-prerelease and I can see that the problem with duplication is gone but a new problem with mixed/not-applied marks is happening. There might be another IME + marks problem on earlier versions which I did not notice.

I'm trying to type twice 你好, once normal and once in bold. I can't get bold active in 1.9.14-prerelease. I need to finish the composition and then bold a piece of text and keep writing again in bold. Sometimes, a composition text is normal and sometimes it's bold. Also, just simple cursor navigation over a text is triggering bold on/off (the last line).

Please check the recordings here (I could not paste the image here as it's somehow not playing the full length):
https://recordit.co/1a16DtR9Io

I see that someone also stumbled on this IME + mark issue:
https://discuss.prosemirror.net/t/chinese-ime-not-work-fine-in-safari-when-use-marks/2090

@marijnh

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Ahh, indeed, that is a problem that's caused by not having cursor wrappers. I'm going to have to think about this more.

@marijnh

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Please try 1.9.14-prerelease2, which adds a kludge where the editor will wrap the cursor in the appropriate marks on compositionstart (if necessary).

@quirm

This comment has been minimized.

Copy link
Author

commented Jul 30, 2019

I tested it on a "basic example" with 1.9.14-prerelease2 as my repo has other problems in my code.

It's "almost" there. When you type "normal bold " and then disable bold and try to type another piece of text, the mark is re-enabled in case of a space in the composition (not sure about the terminology here). I need to disable bold beforehand and start the composition without undesired mark, then it works as expected.

Safari left; Chrome right
image

Isolated problem with marks in Safari:
image

Thanks a lot for fixing the original issue. I will close this as the original problem has been resolved. I'm happy to do further testing/debugging if you have other changes for the mark issue.

@quirm quirm closed this Jul 30, 2019

@marijnh

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Thanks for the feedback. Do I understand correctly that the problem occurs when you try to enable/disable marks during a composition? If so yes, that isn't handled yet—and can't be handled without interrupting the composition, since the editor can't touch the DOM that's being composed without interrupting. But maybe interrupting is the right thing to do there. What would be the expected behavior for you?

@quirm

This comment has been minimized.

Copy link
Author

commented Jul 30, 2019

Steps to reproduce:

  1. type normal text "你好 "
  2. enable bold
  3. type bold text "你好 "
  4. disabled bold
  5. type normal text "你好"

Once you type pinyin ni for the first character and start typing further for the second character, then the mark is automatically re-enabled.

If the bold mark is disabled and then you start typing, it will be re-enabled during the composition in case of space. But, if there are no marks, e.g. disabled a character before, it will work as expected.

Another gif showing this issue. The first line has the bold mark disabled 1 character before, the second line has the bold disabled just before another keypress starting the composition. I'm only clicking on the bold toolbar button before the finished composition, not in the middle, no key shortcuts.
image

@marijnh

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

Which Chinese input method are you using, precisely? I wasn't able to get those characters for 'ni hao', and I'm not seeing the bug happen.

@quirm

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

I'm using "Pinyin - Simplified", Safari v12.1.2.
I also tried Safari iOS and it works fine.

@marijnh

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Thanks. I can reproduce the issue with that IME. It seems it does something odd on 'word' boundaries that causes the whole composition to revert to the style of its context. I can reproduce the same behavior in a non-ProseMirror contenteditable element, and I'm not sure how to work around it. Filing an issue with Safari might eventually help, if you're lucky (just going through the thing you showed in an editable element that has some key combo wired up to document.execCommand("bold") should provide a demonstration case). I'm, unfortunately, out of ideas on how to fix this inside ProseMirror.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.