-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Windows] Text suggestion doesn't replace the whole word #13583
Comments
Hi @Czhang0727, thank you for the report. I've edited your post to summarize steps required to reproduce the problem. Below I'm sharing debugging notes from the Microsoft team: Events fired after replacing "al" -> "also"
Potential source of the problem
|
Some context: The Unfortunately, we cannot blindly use
Bonus: We are aware of an issue with Grammarly where accepting a suggestion to remove an unnecessary word doesn't delete that word. Potentially, it's the same scenario. The idea is to try to come up with a heuristic that uses the target range provided by the browser when it's related to spell-checking scenarios but use our logic in other cases. |
After a quick investigation, looks like Grammarly uses a different event : Checked with Let's not focus on it right now, and fix |
This got even more complicated. We would like to delete a single code point on the backspace key press, and the whole character on the delete key press. Also no matter the direction the whole emoji should be removed. In the above screencast, you can see that behavior of the backspace key press after a combined symbol (for example There was a workaround for Android that would detect simple ranges but it would not handle combined symbols properly (as described above). I propose a heuristic that would detect if the target range is covering more than one character (combined symbols, and emojis count as a single character, also elements like image or BR count as a character).
Doubts:
This is still a work in progress but looks promising: #13685 |
@niegowski I tried to re-edit your comment as some bits of it were unclear to me. Please check out the diff whether I understood all correctly. Also, two comments:
|
The target range actually ends/starts inside a widget, crosses non-editable elements, etc. But this has no visible effect because
I was thinking about how to bulletproof those ranges. I realized that we need to fix those ranges before the There is also another option. We could listen to the |
You mean a round-trip from a view range -> model range -> fix model range (same as the current selection postifxer) -> view range? This sounds interesting. Two thoughts:
|
exactly, but view range update only if model range required any fixing
I think it should not affect what is deleted. The fixed range is more reasonable because it includes the whole object and not just a part of it.
Nope, in special cases we listen to
I'm not sure, shall I do some performance tests? As I noted here:
The current proposal is fixing all target ranges in beforeInput events so also could affect typing but seems more reasonable to fix it there too so that typing over a non-collapsed selection would make sure that the range is valid. |
I forgot to comment on that. I like this option much more than the idea of targeting only the
I don't know the exact algorithm that we implement in the post-fixer. Do we always extend the range? I recall issues navigating up and down with shift+arrow keys due to that post-fixer so that made me a bit worried here.
I think so.
I think I need to test the PoC myself because I forgot too much of the execution flow for these cases :D |
I did some performance tests (using the performance tab in Chrome dev tools and by measuring the time of a single |
Selection post-fixer flow:
|
I think it should not cause any issues because the current implementation (on master) is using the document selection that is already fixed by the selection post-fixer so it looks like acting on the same range.
Is this what you are mentioning? Looks like the selection post-fixer would need some user intentions to do it better, now it does not know if the user intention is to expand or to shrink the selection. |
My point was: does the outcome of the post-fixer make sense for "we want to delete that range" in the context of where Backspace was pressed. But I suppose this needs to be tested manually. I don't question here the post-fixer itself. Nor whether the range will be "correct" (whatever it means). But the integration of all these pieces when handling Backspace. |
I found this page by asking ChatGPT about this problem. I thought to add that this is a windows wide issue. E.g. in Microsoft Teams I type predi - and I choose "predict" - it will add the suggestion next to what I already typed, which is, as you can see - a problem. It looks like this is what we see in action in this issue and it itself is not a ckeditor issue. Unless.... MS Teams uses CKeditor :-)
edited: added links to Microsoft answers. |
😉 |
i have the same problem wit my windows on any app. |
I also have the same problem in my windows 11 pc on any application, even with notepad, all MS Office applications, save file dialog etc. |
📝 Steps to reproduce
Current result
The text replaces only the last character from the word, instead of the whole word.
Original message
Currently we see windows text suggestion has issue on CKEditor for "all suggestions will get inserted instead of replace"
Please find feature flag here:
Settings > Time & Language > Typing > Show text suggestions when typing on the physical keyboard.
Please note this issue currently only reported on Ckeditor5.
What is the expected behavior of the proposed feature?
The text will get replaced by suggestions.
If you'd like to see this feature implemented, add a 👍 reaction to this post.
The text was updated successfully, but these errors were encountered: