[Windows] Text suggestion doesn't replace the whole word

[Czhang0727]

📝 Steps to reproduce

1. Enable text suggestions in Windows 10/11 (https://support.microsoft.com/en-us/windows/enable-text-suggestions-in-windows-0bf313ca-c992-4173-aa5f-8341d3953498).
2. Go to https://ckeditor.com/docs/ckeditor5/latest/examples/builds/classic-editor.html.
3. Type "thi".
4. Select "this" from the auto-complete box.

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.

[Mgsy]

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"

• The trace above is related to the al->also replacement text. 
• Upon after the keypress to type l (to get al), the “also” suggestion is returned. 
• After selecting “also”, the beforeinput (deleteContentBackward) event is fired, followed by a series of input and composition events.
• Inspecting the event data, the target ranges included in the event data payload are seemingly correct, but somehow when handling the deletion, the characters are not being removed correctly.

Potential source of the problem

• The potential source of the problem lies in the following line of the code: https://github.com/ckeditor/ckeditor5/blob/0e5ddd74e5684897d2d2c87a0e65150c287433d4/packages/ckeditor5-typing/src/deleteobserver.ts#L173
• This condition is being gated by env.isAndroid and is causing the deletion code to not run properly.
• The deletion is only removing the last character of the range to be deleted
• By removing the env.isAndroid in the condition evaluation, this code runs and behaves appropriately when replacing the chosen suggestion text with the old text

[Reinmar]

Some context: The deleteContent(|Backward|Forward) events spec is unclear and in mooost cases we didn't need to use targetRange because our internal calculations of what should be removed was sufficient. It's also actually tricky to calculate what needs to be removed – depending on the direction it's a code point or character and emojis have very custom handling.

Unfortunately, we cannot blindly use targetRange in all cases:

• we don't know whether every browser and possible external integrations provide correct target ranges,
• the target range may be incorrect in cases like merging blocks or next to block non-editable objects.

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.

[Witoso]

After a quick investigation, looks like Grammarly uses a different event :keydown (key:"Delete").

Checked with beforeinput test with a phrase Leaving your comfort zone might lead you to such beautiful sceneries like this one.. The word such is suggested for deletion by Grammarly.

Let's not focus on it right now, and fix deleteContentBackward scenario.

[niegowski]

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 b̂ or ã) in the plain contenteditable differs from what the browser reports in beforeInput.targetRanges. We should detect what is inside the target range and decide whether we want to use it or use model logic to determine the range of deletion.

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).

• If the target range is covering more than one character then the target range should be used.
• If the target range is one-character long (or even shorter, making the heuristic bulletproof when browsers start treating code points granularly) then the standard model-based logic should be used (to remove a single code point instead of a whole combined symbol).

Doubts:

• What if the caret is inside an inline filler?
  • The DOM position is mapped to the position just before an inline filler in the view. So, no problem here, since we are dealing with this on the view level (no inline filler here).
• What if the range includes a single character and an inline element?
  • That's why I'd count elements as characters to make sure that the proper range is removed (and for consistency).
• What if the range ends or starts inside a widget?
  • We can safely use such a target range (delete it), because those positions will properly map to correct model positions.
• What if the backspace key is pressed while some widget is selected (fake selection container)
  • We do not use target ranges in this case, target ranges are used straight from the fake selection.
• We don't know whether every browser and possible external integrations provide correct target ranges.
  • We should check it but if the target range is not provided then the whole logic should fall back to using the model selection.
• The target range may be incorrect in cases like merging blocks or next to block non-editable objects.
  • This should be tested but while playing with the proposed solution I did not notice any problems.

This is still a work in progress but looks promising: https://github.com/ckeditor/ckeditor5/pull/13685

[Reinmar]

@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 heuristic must be bulletproofed. We cannot expect that target ranges will follow any particular rules as they may be provided by a browser (and change with its new versions) or external tools (e.g. spellchecks). So let's not assume anything about those target ranges.
• "The target range may be incorrect in cases like merging blocks or next to block non-editable objects."
  • How does the heuristic work in this case?

[niegowski]

• What if the range ends or starts inside a widget?

• We don't know whether every browser and possible external integrations provide correct target ranges.

• The target range may be incorrect in cases like merging blocks or next to block non-editable objects.

The target range actually ends/starts inside a widget, crosses non-editable elements, etc. But this has no visible effect because delete event handlers use bubbling event propagation or document selection to handle delete actions other than triggering the delete command. However, this seems fragile and might break easily especially since the bubbling starts from the target range.

• The heuristic must be bulletproofed. We cannot expect that target ranges will follow any particular rules as they may be provided by a browser (and change with its new versions) or external tools (e.g. spellchecks). So let's not assume anything about those target ranges.

I was thinking about how to bulletproof those ranges. I realized that we need to fix those ranges before the delete event is fired (because the bubbling event could be intercepted by some handlers - for example, the widget could stop the event and handle it differently than the delete command). But the delete event is fired by the observer that does not have access to the model and that range could be fixed only on the model level. I realized that we already fire beforeinput event by another observer and the delete observer is acting on that beforeinput event. So my proposal is to listen to beforeinput event fired by InputObserver and fix the targetRanges according to the model and model schema rules. We already have a selection post-fixer so we could reuse that logic to fix target ranges and update event if fixing is needed. (this is already implemented in the PR: https://github.com/ckeditor/ckeditor5/pull/13685/commits/f6c1b6ba669cedb55539dec1a7fc13024db3a674)

There is also another option. We could listen to the delete event in the $capture phase and fix the target range at that stage but this would not fix possible issues with other beforeinput ranges (insertText, enter, etc)

[Reinmar]

So my proposal is to listen to beforeinput event fired by InputObserver and fix the targetRanges according to the model and model schema rules. We already have a selection post-fixer so we could reuse that logic to fix target ranges and update event if fixing is needed.
(this is already implemented in the PR: f6c1b6b)

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:

• Since we start with a weird view range (e.g. crossing some cE boundaries), I wonder how will the fixed range look like. I guess sometimes it can  be severely distorted when compared to the target range that we got. Do you think it may affect e.g. deleting in some cases? Since we'll be using the target range when it's covering more than one character, it will drive how the backspace works. Does it mean that we'd give the browser (distorted by our post-fixer) complete control on that action?
• I wonder what's the performance implication. Test case: long paragraph with a lot of inline formatting and keeping the backspace pressed.

[niegowski]

You mean a round-trip from a view range -> model range -> fix model range (same as the current selection postifxer) -> view range?

exactly, but view range update only if model range required any fixing

• Since we start with a weird view range (e.g. crossing some cE boundaries), I wonder how will the fixed range look like. I guess sometimes it can  be severely distorted when compared to the target range that we got. Do you think it may affect e.g. deleting in some cases? Since we'll be using the target range when it's covering more than one character, it will drive how the backspace works. Does it mean that we'd give the browser (distorted by our post-fixer) complete control on that action?

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.

• Does it mean that we'd give the browser (distorted by our post-fixer) complete control on that action?

Nope, in special cases we listen to delete bubbling event that starts bubbling from the target range and we intercept it for example on widgets or next to those.

• I wonder what's the performance implication. Test case: long paragraph with a lot of inline formatting and keeping the backspace pressed.

I'm not sure, shall I do some performance tests?

As I noted here:

There is also another option. We could listen to the delete event in the $capture phase and fix the target range at that stage but this would not fix possible issues with other beforeinput ranges (insertText, enter, etc)

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.

[Reinmar]

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 delete event. The latter sounds more like a hack.

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.

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'm not sure, shall I do some performance tests?

I think so.

• Does it mean that we'd give the browser (distorted by our post-fixer) complete control on that action?

Nope, in special cases we listen to delete bubbling event that starts bubbling from the target range and we intercept it for example on widgets or next to those.

I think I need to test the PoC myself because I forgot too much of the execution flow for these cases :D

[niegowski]

I'm not sure, shall I do some performance tests?

I think so.

I did some performance tests (using the performance tab in Chrome dev tools and by measuring the time of a single beforeinput action and continuous action of holding a key and checking the time it took to delete a paragraph with mixed inline elements) and I can't see any performance issues.

[niegowski]

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.

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.

Selection post-fixer flow:

• Is collapsed range?
  • Ask Schema#getNearestSelectionRange() for help.
  • Check the surrounding of the position (without leaving the Schema#isLimit()) and use the first position that accepts the text (Schema#checkChild()) or the range on the closest found Schema#isObject().
  • If none then find the nearest Schema#isObject() ancestor of the position and use the range on it (this is the case when the range was inside a limit element that do not accept text inside).
• Is non-collapsed range?
  • Is the range in a single limit element?
  • If some of the range's ends do not accept text (Schema#checkChild()) and are next to a block element (not a limit and not selectable)
    • Ask Schema#getNearestSelectionRange() for help to fix both range's ends (but inside the original range). So the selection is not between blocks and tries to stick to the closest text or selectable element ([<block>foo</block>] -> <block>[foo]</block>)
  • Else the range spans different limit elements (crosses limit element boundaries)
  • Find the common Schema#isLimit() ancestor for both range's ends and use the range on it. This is using the outermost limit element for directly nested limit elements like in table > tableRow > tableCell.

[niegowski]

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 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.

I recall issues navigating up and down with shift+arrow keys due to that post-fixer so that made me a bit worried here.

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.

[Reinmar]

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.

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.

[pkhodo]

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 :-)

• https://answers.microsoft.com/en-us/msteams/forum/all/text-prediction-working-incorrectly-in-teams-chat/7865ad31-e00f-4df8-b07d-1cc768f68a6b
• https://answers.microsoft.com/en-us/msteams/forum/all/autocorrect-is-malfunctioning-not-replacing-the/6dd65f46-0106-4d87-8d63-75a84849d1b7

edited: added links to Microsoft answers.

[Reinmar]

Unless.... MS Teams uses CKeditor :-)

😉

[soheilamiri]

i have the same problem wit my windows on any app. OS: 10.0.22635

[princejosephfelix]

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.