[Android-Samsung keyboard] Enter two or three spaces quickly, then the editor writes backwards

[uaza]

📝 Provide detailed reproduction steps (if any)

1. Open any CKEditor demo, e.g.https://ckeditor.com/ckeditor-5/demo/mobile-support/
2. Type a word and then either two or three spaces. Depending on the Android device, you may need two or three spaces for the problem to occur.
3. Keep typing, the editor will insert the text backwards.

✔️ Expected result

The editor inserts the text in the correct order.

❌ Actual result

The editor insert the text backwards. Subsequently, deleting the text also does not work properly and skips some letters.

📃 Other details

• Browser: Chrome
• OS: Android 10 + 12
• First affected CKEditor version: 36.0.1 (34.0.0 was fine)
• Installed CKEditor plugins:
  Alignment,
  AutoImage,
  Autoformat,
  AutoLink,
  BlockQuote,
  Bold,
  Code,
  CodeBlock,
  Essentials,
  FontBackgroundColor,
  FontColor,
  FontFamily,
  FontSize,
  Heading,
  HorizontalLine,
  Image,
  ImageCaption,
  ImageInsert,
  ImageResize,
  ImageStyle,
  ImageToolbar,
  ImageUpload,
  Indent,
  IndentBlock,
  Italic,
  Link,
  LinkImage,
  List,
  ListProperties,
  MediaEmbed,
  Mention,
  Paragraph,
  PasteFromOffice,
  RemoveFormat,
  Strikethrough,
  Subscript,
  Superscript,
  Table,
  TableCellProperties,
  TableProperties,
  TableToolbar,
  TableCaption,
  TableColumnResize,
  Title,
  TextTransformation,
  TodoList,
  Underline

android_typing_backwards.mp4

---

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

[Witoso]

@ckeditor/qa-team could you check this behavior on the newest version?

[Acrophost]

@Witoso I can confirm it still reproduces on latest docs (checked here: https://ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.html, on Chrome@Android 11)

[mabryl]

I just retested this in the Mobile friendly editor demo and in our documentation the issue is no longer reproducible.

Here's my testing setup:

Device: Samsung Galaxy A52s 5G
OS: Android 13
Keyboard: Gboard v13.2.04
Browsers: Chrome 116.0.5845.163 / Firefox 117.0

Could you try reproducing the issue now @uaza? Recently, we've been able to confirm that a couple of other issues regarding typing on Android have been fixed and it looks like this one is fixed as well.

[mabryl]

Here's a recording of the issue no longer being reproducible:

android_typing.mp4

[uaza]

Thank you for testing. I just retested the bug with the "Mobile friendly editor demo" as well and it is still reproducible on my end. Based on your description, I was able to determine the difference and the cause. You use the "Gboard" keyboard, I use the Samsung keyboard. Also, the Samsung keyboard has the "Predictive text" switch enabled in the keyboard settings. If you turn off this switch, the bug does not occur.

I hope this information helps you to narrow down and fix the bug.

[uaza]

One addition that might be interesting. If you have reproduced the bug with the Samsung keyboard once and then switch to the GBoard keyboard, the input does not work properly anymore. Only a reload of the page eliminates the bug again.

[dtdesign]

While working on #14567, I came around a second type of issue that I dubbed the “double duplication bug”. Typing after two consecutive spaces causes the typed-in character to appear twice to the right of the caret. The next keystroke will again be duplicated twice to the right, with the content in front of the caret remaining unmodified. Every keystroke afterwards will behave as normal.

On the surface this sounds like something completely different but after some testing I noticed something that makes be believe that this is effectively just a different symptom of the same exact problem. I’m using a modified version of input.ts (part of ckeditor/ckeditor5-typing) with my workaround being applied plus some extra debugging messages to report the three composition events. My test setup is the string Test | (with | representing the caret) and once you type in t, it will yield Test |tt. The output looks like this:

// Editor contains `Test  |`
// `t` is being pressed
compositionstart: data -> <empty string>
compositionupdate: data -> t
insertText: text -> t (selectedText being resolved as <empty string>)
insertText: text -> t (selectedText being resolved as <empty string>)
compositionend: data -> t

Notice that the insertText event is triggered twice, but there is no compositionupdate being dispatched. This by itself isn’t anything out of the ordinary, because insertText will regularly be triggered outside of a composition, for example, when entering a space. The important bit here is that the second invocation of insertText happens inside a composition but without a compositionupdate taking place.

Using Composition Events to Suppress “Echo Events”

I then set up a logic to report and suppress the insertText event whenever it is called inside a composition but without being preceeded by a compositionupdate. It seems that this does happen every now and then while typing regularly, but is some kind of “extra” event, as if this was some kind of echo of the first one.

Interestingly, when I suppress those “echo events”, the “double duplication bug” results in this:

Test  |
// Entering `T`
Test__|T
// Entering `e`
Test  T|e
// Entering `s`
Test  Te|s
// Entering `t`
Test  Te|ts

Every keystroke afterwards is added to the left of the caret.

The eagle-eyed among you might have spotted the two underscores present in line 3. This is something that I was previously only able to witness with a debugger breakpoint inside the insertText listener. The underscores represent the visible composition in the editor that usually only appears when the browser is offering suggestions. It actually just underlines those two spaces, nothing else.

One interesting tidbit: When I disable the suppression of those “echo events” and instead use a debugger breakpoint instead, I get the same exact behavior. The “echo event” never takes place when the debugger is active which is a class of heisenbugs I have witness countless times over the years when using breakpoints with contenteditable. Nevertheless, it provides some insight into what is actually happening here.

How This Is Related to Reverse Typing

Coming back to the original issue: The reverse typing effect is a bit inconsistent, sometimes I face an even worse variant where every now and then a character appears to the left of the caret. It is worse because the next keystroke will add the keystroke to the right plus whatever is to the left of the caret. And yes, the string to the left of the caret eventually grows so that the amount of characters inserted to the right increases over and over.

However, as soon as I enable the “echo event” suppression, I am no longer able to reproduce the reverse typing effect and even its evolved variant does not take place. Except, if I spam keystrokes really hard, it rarely triggers the reverse typing bug for a single letter.

Final Thoughts

Quite frankly, I do not (yet) understand the nature of those extra events and what their purpose are. It feels an awful lot like I’m missing a puzzle piece here that helps me wrap my head around the composition behavior. Also, I have not yet tested how those suppressed events affect the functionality of the editor, including “external” data injections like paste events.

[dtdesign]

Thankfully I looked into this issue too because I can only reliably reproduce this in Chrome on Android. This is closely related to the “double duplication bug” discovered in #14567, because the reverse typing is caused by a mismatch of the selection. Please read #14567 (comment) to understand what is happening, in particular the importance of the selectionchange event which is the root cause of the caret repositioning.

Eventually I was able to abuse Chrome’s performance profiler to figure out what causes the selection change, narrowing it down to the view renderer. One of its jobs is to sync the DOM with the expected content which for text nodes is done through updateTextNode(). When everything is working correctly, the function is invoked once per keystroke and the call stack looks identical.

Things get wild when the “reverse typing” (Chrome on Android) or “double duplication” (Firefox on Android) are being triggered. Everything looks normal until suddenly the MutationObserver kicks in and causes two additional updates to the DOM which is eventually followed by a selectionchange. This also explains why I previously did not figure out the cause of the selectionchange event because mutation observers are async.

My actual logging output is a bit more complex because I’m monitoring a lot of different states but the gist of it is this:

Input::insertText("t")
 Range is (6:6)
 Inserted text "t"
DocumentSelection::_setTo(7:7)
Renderer -> updateTextNode(type=insert), actual = "Test  ", expected = "Test  t"
 Renderer @ insert "t"
Renderer -> updateTextNode(type=insert), actual = "Test  tt", expected = "Test  t"
 Renderer @ insert "  "
Renderer -> updateTextNode(type=delete), actual = "Test  tt", expected = "Test  t"
 Renderer @ delete x3 
DocumentSelection::_setTo(6:6)

The two updateTextNode() calls at the end are the problem, they appear to be completely out of sync. Notice how the first one of them attempts to insert a single whitespace that comes out of nowhere and is the result of the DOM mutation event.

Out of curiosity I added a switch to block the forced render inside the observer and suddenly the behavior is similar to #14567 with the difference that the last typed character appears to the right of the caret. It is different in that this is constantly updated and is not strictly the first character. So there is some merit to the mutation events, it’s just that they seem to sometimes report something that throws off the internal diff.

[dtdesign]

So, I did check what causes the difference and it looks like the issue is caused by a mismatch of the white spaces. I did not understand how comparing "Test tt" with "Test t" resulted in a whitespace being added, until I replaced \u00a0 with a substitute character (here: █). It is actually comparing Test█ tt with Test █t which results in a difference found at index 4.

I have the suspicion that there is some kind of normalization at work that favors one way over the other but breaks with the browser’s expectations. fastDiff() by default just does a strict comparison of the inputs a and b so I just tried passing in a custom cmp function in updateTextNode() which normalizes \u00A0 to (0x20) for the sake of simplicity, effectively allowing the two strings to drift apart for as long as its only the spaces.

drumrolls

IT WORKS. 🎉

diff --git a/packages/ckeditor5-engine/src/view/renderer.ts b/packages/ckeditor5-engine/src/view/renderer.ts
index 7fa8cdff61..ccee1fe33d 100644
--- a/packages/ckeditor5-engine/src/view/renderer.ts
+++ b/packages/ckeditor5-engine/src/view/renderer.ts
@@ -1230,7 +1230,13 @@ function updateTextNode( domText: DomText, expectedText: string ) {
 	// @if CK_DEBUG_TYPING // 	);
 	// @if CK_DEBUG_TYPING // }
 
-	const actions = fastDiff( actualText, expectedText );
+	const actions = fastDiff(
+		actualText,
+		expectedText,
+		( a, b ) => {
+			return a.replace( /\u00a0/g, ' ' ) === b.replace( /\u00a0/g, ' ' );
+		}
+	);
 
 	for ( const action of actions ) {
 		if ( action.type === 'insert' ) {

This fixes both the “double duplication” as well as the “reverse typing”. And it doesn’t stop here, I removed my workaround from #14567 (comment), thus the only thing that was left is that tiny little change above and it still works. All three types of bugs fixed with a single normalization of the white spaces during the change detection.

Thinking about it, this totally makes sense. Read my previous message in this issue, in particular the part about the composition events where I noticed an odd placement of the composition indicator Test__|T. The browser was indirectly hinting the issue the whole time, the diverging types of spaces caused a modification of the DOM text at an unexpected location (index=4) which causes the underline to appear at that position.

@Witoso @Reinmar I’m not sure if I should submit a PR at this point because I don’t understand the implications of my change above, in particular how much of an issue it is that the white spaces in the DOM and the model differ. I’m looking forward to test this change on our site, but I would really love some feedback from you guys considering how severely these two bugs impact the editing experience on Samsung Android phones.

As before, I hereby release my above changes into the public domain, do whatever you want! ;-)

[ghost]

Genius! @dtdesign respect! Thank you!

[dtdesign]

It looks like I celebrated a bit too early, at least partly. The normalization is an important part because it prevents \u00a0 to be smuggled in between words where a regular space (0x20) should be inserted. We’re currently running the patched version on our site and it displayed an odd behavior on desktop where lines would wrap somewhat arbitrarily as a result of those sneaky \u00a0 yielding extremely long words. What makes them confusing that the spaces look identical so users are confused to what is happening.

One possible solution would be to avoid the normalization in places that are not nearby the current composition or at least take place outside of a composition. This would allow the conversion to 0x20 where it is reasonable without throwing the browser off-guard.

I’m planing to deploy a slightly modified version which applies the change to Android devices only, because the impact of the missing normalization is a somewhat acceptable tradeoff on mobile devices. Having lines sometimes wrap at an unexpected point is bad but is the lesser evil when compared to the current situation which severely breaks typing altogether.

[dtdesign]

Restricting the workaround to env.isAndroid works just fine and apparently \u00a0 is not an issue on Android, perhaps because of the different handling introduced in #12446. In hindsight I should have added this condition in the first place because typing on Android is wildly different.

This whole issue feels an awful lot like the Hydra because whenever I fix an issue a new one pops up. When I am backspacing words, the caret jumps to the end of the previous word even when there is still a trailing space. Hitting backspace once will not move the caret, but remove the space. Hitting backspace once more removes the last letter of the word.

For illustration:

1, Test te|
2. Test t|
3. Test|
4. Test|
5. Tes|

This is quite annoying and I think it has to do with the regular space (0x20) being inserted, but the caret on Android will only display to the right of the space when it is a \u00a0. Disabling the normalization appears to break this. Yay.

Considering what I have learned in my journey over the past week, I am somewhat confident that #11569 is caused by the same exact problem and should be fixed by my change, because the normalization breaks the composition process.

In addition, I am no longer able to reproduce #14707 with my fix, which indicates that this was indeed once more a case of a mismatch caused by the normalization of the space character.

I would love to see some input from @niegowski who worked through the pain of implementing the IME support on Android in #12446 in the first place. The visual misplacement of the caret when backspacing towards a trailing regular space appears the only remaining issue, but my experience with Android is very limited.

For reference, this is my current change that I am using:

diff --git a/packages/ckeditor5-engine/src/view/renderer.ts b/packages/ckeditor5-engine/src/view/renderer.ts
index 7fa8cdff61..54aff6e7b1 100644
--- a/packages/ckeditor5-engine/src/view/renderer.ts
+++ b/packages/ckeditor5-engine/src/view/renderer.ts
@@ -1230,7 +1230,17 @@ function updateTextNode( domText: DomText, expectedText: string ) {
 	// @if CK_DEBUG_TYPING // 	);
 	// @if CK_DEBUG_TYPING // }
 
-	const actions = fastDiff( actualText, expectedText );
+	const actions = fastDiff(
+		actualText,
+		expectedText,
+		( a, b ) => {
+			if ( !env.isAndroid ) {
+				return a === b;
+			}
+
+			return a.replace( /\u00a0/g, ' ' ) === b.replace( /\u00a0/g, ' ' );
+		}
+	);
 
 	for ( const action of actions ) {
 		if ( action.type === 'insert' ) {

[dtdesign]

I wasn’t satisfied with the misplaced caret when the text ends with a regular space (0x20) so I changed the logic by attempting to detect a single character backspacing event where the caret would now be right after a regular space, but the browser (?) wants it to be \u00a0.

So far it works perfectly, preserving the composition when typing and avoiding all sorts of bugs associated with the spaces. In contrast it is now able to fix the visual caret position by allowing changes that would replace a trailing regular space with \u00a0. Admittedly this is merely a workaround.

diff --git a/packages/ckeditor5-engine/src/view/renderer.ts b/packages/ckeditor5-engine/src/view/renderer.ts
index 7fa8cdff61..30b9d9d0f9 100644
--- a/packages/ckeditor5-engine/src/view/renderer.ts
+++ b/packages/ckeditor5-engine/src/view/renderer.ts
@@ -1230,7 +1230,24 @@ function updateTextNode( domText: DomText, expectedText: string ) {
 	// @if CK_DEBUG_TYPING // 	);
 	// @if CK_DEBUG_TYPING // }
 
-	const actions = fastDiff( actualText, expectedText );
+	let diff = ( a: string, b: string ) => a === b;
+	if ( env.isAndroid ) {
+		// This is a very crude attempt to detect backspacing at the end of the text.
+		if (
+			actualText.length > 1 &&
+			actualText.length - 1 === expectedText.length &&
+			expectedText.endsWith( '\u00a0' ) &&
+			/ .$/.test( actualText )
+		) {
+			// Do not make any change, we *want* the `\u00a0` to appear at the end.
+		} else {
+			diff = ( a, b ) => {
+				return a.replace( /\u00a0/g, ' ' ) === b.replace( /\u00a0/g, ' ' );
+			};
+		}
+	}
+
+	const actions = fastDiff( actualText, expectedText, diff );
 
 	for ( const action of actions ) {
 		if ( action.type === 'insert' ) {

This stuff happens when people leave me unsupervised … 😉

[dtdesign]

We’re shipping this change since February 22nd and it has proven to fix this and similar issues.

However, depending on the browser and keyboard this has the unwanted side-effect of preserving \u00a0 all over the place which obviously is an issue with implicit line breaking. This could be addressed by normalizing the whitespaces that are not nearby the current selection to prevent breaking the selection handling.

@Witoso I’m a bit concerned about the lack of feedback from you guys. I mean I get it, these things take time but this is an almost 1 year old bug report that severely impacts the writing experience on Android. It’s not like this is an isolated case for a niche feature, we’re talking about messing up everything when typing two consecutive spaces which can easily be the result of “fat-fingering” the space bar.

Some of these bugs are reproducible with Google Chrome and GBoard, it doesn’t get more standard than that. Even in Poland the stats read something like 80%+ marketshare of Android, I really don’t understand how this gets prioritized so low.

Your last feedback about this kind of issue was in December stating that you need to source some testing devices but that was is it. In early February we simply ordered a cheap Android phone from Amazon and started digging into this issue plus finding a (somewhat reasonable) workaround. We even had to dig deep into the inner workings of CKEditor to even understand what is going on, it should take experienced developers like you have a fraction of the time to get to this point.

If you need more time to figure out a solution then at least let everybody know, or if you need more help then just speak up, this is an open source project after all! 🙂

[Witoso]

Hi! Thanks for the ping, I’m glad the fix worked for you and solved the issue for your app. We will for sure take a look at your research.

I don’t want to go into the weeds of the prioritizing things on our side. The typing has a massive splash zone of risk, and we deliberately want to schedule time for this. Even reading and analyzing your proposed solution would take significant time. But we do discuss it internally as we are aware of the impact.

Another project that has considerable impact on the project, and is in progress, is #15502, defocusing from it right now would be too risky to do. We decided to go fully into it, finish, and only then move to other things.

We will update this issue accordingly when we start working on it.

[Witoso]

We are taking a look at the #16106 issue, although unrelated, we plan to spend time on investigating this one as well.

[winzig]

OK @Witoso #15502 is done now, when will this major keyboard bug get worked on?

[Witoso]

@winzig the work for this issue and others related is completed in the #16289, and we finalized the round of internal (successful) tests. I encourage everyone to test the PR if you have a chance. It should be merged in the following days, and will be a part of the next release.