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

[IME] Typing after inline widget breaks the composition #5877

Closed
farthinker opened this issue Dec 2, 2019 · 9 comments
Closed

[IME] Typing after inline widget breaks the composition #5877

farthinker opened this issue Dec 2, 2019 · 9 comments
Labels
domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). resolution:expired This issue was closed due to lack of feedback. status:stale type:bug This issue reports a buggy (incorrect) behavior.

Comments

@farthinker
Copy link

📝 Provide detailed reproduction steps (if any)

  1. Visit the demo in inline widget tutorial: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/tutorials/implementing-an-inline-widget.html#demo
  2. Press backspace until the selection is right after the inline widget
  3. Insert a space(It's important!) and then start typing with Chinese IME

✔️ Expected result

Typing Chinese words correctly.

❌ Actual result

Composition breaks.

📃 Other details

  • Browser: Chrome 78.0.3904.97
  • OS: Mac OS 10.15.1
  • CKEditor version: CKEditor5 latest version
  • Installed CKEditor plugins: nothing special

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@farthinker farthinker added the type:bug This issue reports a buggy (incorrect) behavior. label Dec 2, 2019
@farthinker
Copy link
Author

farthinker commented Dec 2, 2019

After some digging, I found browser would sometimes insert "breaking" whitespace (char code 32) instead of "non breaking whitespace" (char code 160) before the mutation event is handled. And CKEditor's editing downcast process will ensure all spaces in the view are "non breaking" whitespaces. This difference will cause sameNodes method return unexpected result, like text node ' t'(whitespace is char code 32) and text node ' t'(whitespace is char code 160) is not the same, which will lead to unexpected DOM rendering and break the composition.

If there's something more I could help please let me know.

@Reinmar Reinmar added domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). status:confirmed labels Dec 3, 2019
@Reinmar Reinmar added this to the next milestone Dec 3, 2019
@Mgsy
Copy link
Member

Mgsy commented Dec 3, 2019

I can confirm the issue. In my case, the additional character appears after typing "we". If the whitespace isn't inserted, it works properly.

bug_cke5

@farthinker
Copy link
Author

It's been more than two months since I became a OEM license client and switch our editor code base to CKEditor 5. Since then almost everyday we could hear complaints about IME issues from our users. It's really sad that most IME issues are ignored or assigned a low priority by the CKEditor's dev team.

I tried to analyze and fix the issues by myself, but there are too many, and most of them are so tricky. I have to spend so much time look into them, and learn to deeply understand the editor core. The result is frustrating: some were fixed, but always more came out.

Today, I decide to try the final solution, ckeditor/ckeditor5-engine#861. I know it's risky and could break collaborative editing, but I'm tired of playing with IME issues everyday. At least most of the IME issues disappear after I stop rendering while composing. Collaborative editing is still an advanced and nice to have feature for us, and since it might introduce more issues, it's unlikely we'll apply this feature in a near feature.

@pjasiun said in ckeditor/ckeditor5-engine#861, it's better to break composition than collaborative editing, or crash the editor in other words. I think it's not totally true, at least for Chinese users. The editor is unusable if the composition keep breaking, it's a vital issue, and it's no better than editor crash.

There's a huge editing market in China. I wish I could recommend CKEditor to friend company with enough confidence. PLEASE pay more attention to IME related issue and the feelings of Chinese users, you won't regret it. @Reinmar

@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2019

I've been monitoring the IME situation and I am aware that the situation is not good. Correct me if I'm wrong, but it looks especially bad on Safari.

As you know, those issue are really tricky to fix. We were aware of that from the very beginning too – our team had a lot of experience with IME even before we started working on CKEditor 5 and I've been actively discussing those issues in W3C. Unfortunately, handling IME is probably the most complex problem with rich-text editing nowadays. Some things are slowly improving, but the whole thing is just a mess.

Anyway, apart from taking IME support into consideration while designing CKEditor 5's engine, so far we approached this topic twice to work on existing issues. These been long attempts, over couple of months in total to work on IME in general and later on IME on Android. We resolved many bugs, but we're still halfway done. And we know that.

The problem is that working on those issues requires long, constant effort. It also requires assigning a core developer with deep knowledge about the engine's architecture. In other words, any attempt at those issues need to be thoroughly planned in advance. We can't just jump on a particular issue in order to try to fix that one thing without considering the wider picture and spending a lot of time on this. Context switching would impede any attempt.

Our plans right now are to at least investigate the problem that occurred recently that forced us to revert your patch. We hope to find a quick solution to that, maybe even some workaround to make sure that we can roll out this patch to as wide audience as possible.

The other thing that we plan is a longer effort, perhaps 3-4 months in 2020 to try to tackle some architectural obstacles or some other approaches. For instance, today it may be possible to switch completely to beforeInput which wasn't possible back in the day when we first implemented support for typing. Naturally, this is not something we can safely done in a month.

Finally, as for the collaboration vs IME comment – our goal is always to avoid having significant regressions. If something work already, it should stay working on a similar or better level forever. You can imagine how frustrating it would be if a feature that worked for months and you used on a production would suddenly stop working. And that was always the case with collaboration and IME. Collaboration works already and IME in a limited way. At this stage, we need to avoid breaking collaboration because it already worked. And the proposed changes were making or could make collaboration completely unusable. The recent change in rendering that you proposed was crashing track changes on Android for everyone. We can't allow that.

I understand your frustration from the IME issues but this is how it is. We just didn't yet had enough time to polish it, taken the complexity of the problem itself and our use case. We know that adoption on the CJK markets suffer because of that and we'll definitely invest heavily in that in the future.

PS. Thank you for your feedback that we got so far from you and the work that you did on the patches. It's invaluable for us to focus our effort on issues from real CJK languages users.

@farthinker
Copy link
Author

farthinker commented Dec 17, 2019

@Reinmar Thank you for your reply with patience. I 100% understand the complexity of IME issues, and I'm not looking forward to fix all the issues in a month. It's no doubt a long-period battle. Both as a community developer and a CJK languages user, I don't want to fight this battle alone. Actually, It's the very feeling when ckeditor/ckeditor5-engine#1783 was rejected and I didn't see any progress in the next two month, and when I post new IME issues and found nobody looked into them except me. All I asks are more updates about the efforts you've made and the next step plan. These information would be very helpful for me to apply some temp fix or workaround.

Before I submitted ckeditor/ckeditor5-engine#1783, I also thought most IME issues happen in Safari. But I found myself wrong when our users continuously feedback IME issue in Chrome and Firefox, like this one and #5895. And ckeditor/ckeditor5-engine#1783 cannot solve them.

Here is most recent working fix I apply: mycolorway/ckeditor5-engine@7d92120, which only prevent DOM changes while composing, and keep the DOM-View mapping updated.

@pomek pomek modified the milestones: next, nice-to-have Nov 10, 2020
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@helptome1
Copy link

so, this question is solved?

@Reinmar
Copy link
Member

Reinmar commented Jul 20, 2022

This issue is solved in the feature branch of #11438 but the fix is not released yet. See the status summary that I wrote in there.

@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 7, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). resolution:expired This issue was closed due to lack of feedback. status:stale type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

7 participants