Skip to content

fix: Fix: Chinese IME Backspace bug。misses first onChange,double-fire…#5985

Open
wljray wants to merge 6 commits intoianstormtaylor:mainfrom
wljray:main
Open

fix: Fix: Chinese IME Backspace bug。misses first onChange,double-fire…#5985
wljray wants to merge 6 commits intoianstormtaylor:mainfrom
wljray:main

Conversation

@wljray
Copy link

@wljray wljray commented Dec 9, 2025

Problem

On Android devices with Chinese IME (Input Method Editor), there's a bug with the delete/backspace behavior:

  1. Bug Scene: Under a Chinese input method, after typing some Chinese characters (directly pressing Backspace without typing is fine; the issue only occurs after typing), the first Backspace doesn't trigger onChange, but the second triggers it twice in a row.
  2. First delete: Does NOT trigger onChange
  3. Second delete: Triggers onChange twice

This causes issues with:

  • Undo/redo history being incorrect
  • State synchronization problems
  • Controlled component behavior being unpredictable

Root Cause

The issue is in android-input-manager.ts:

Issue 1: storeDiff doesn't schedule flush

When deleteContentBackward stores a diff via storeDiff(), the diff is stored but never flushed. The flush only happens when:

  • scheduleAction() is called (which sets setTimeout(flush))
  • compositionEnd event fires
  • Other specific events

So the first delete stores a diff but doesn't trigger flush → no onChange.

Issue 2: Duplicate onChange on flush

In the flush() function, after processing diffs:

  1. Editor.deleteFragment() is called → triggers onChange via apply()Promise.resolve().then()
  2. At the end of flush(), editor.onChange() is called again if userMarks !== undefined

Since editor.marks can be null, and null !== undefined is true, this causes a duplicate onChange call.

Solution

Fix 1: Schedule flush after storing diff

Add scheduleFlush() calls at the end of storeDiff() to ensure stored diffs are processed and trigger onChange:

const storeDiff = (path: Path, diff: StringDiff) => {
  // ... existing code ...
  
  if (idx < 0) {
    // ... existing code ...
    updatePlaceholderVisibility()
    scheduleFlush()  // NEW: Schedule flush after storing
    return
  }

  // ... existing code for merging ...
  scheduleFlush()  // NEW: Schedule flush after updating
}

Fix 2: Avoid duplicate onChange

Only call editor.onChange() when marks actually changed:

const userMarks = EDITOR_TO_USER_MARKS.get(editor)
EDITOR_TO_USER_MARKS.delete(editor)
// Only call onChange if marks actually changed
if (userMarks !== undefined && userMarks !== editor.marks) {
  editor.marks = userMarks
  editor.onChange()
} else if (userMarks !== undefined) {
  // Just restore marks without triggering another onChange
  editor.marks = userMarks
}

Testing

Steps to reproduce (before fix):

  1. Open Slate editor on Android device (or Chrome DevTools mobile emulation)
  2. Type some Chinese characters using IME
  3. Press backspace/delete once → No onChange triggered
  4. Press backspace/delete again → Two onChange events triggered

After fix:

  1. Each delete triggers exactly one onChange event

Checklist

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • I have added tests to cover my changes (manual testing on Android)
  • All new and existing tests passed

ChangeList

packages/slate-react/src/hooks/android-input-manager/android-input-manager.ts

Diff

diff --git a/packages/slate-react/src/hooks/android-input-manager/android-input-manager.ts b/packages/slate-react/src/hooks/android-input-manager/android-input-manager.ts
index xxx..xxx 100644
--- a/packages/slate-react/src/hooks/android-input-manager/android-input-manager.ts
+++ b/packages/slate-react/src/hooks/android-input-manager/android-input-manager.ts
@@ -239,9 +239,14 @@ export function createAndroidInputManager({
 
     const userMarks = EDITOR_TO_USER_MARKS.get(editor)
     EDITOR_TO_USER_MARKS.delete(editor)
-    if (userMarks !== undefined) {
+    // Only call onChange if marks actually changed, to avoid duplicate onChange calls
+    // The apply() calls above already trigger onChange via Promise.resolve().then()
+    if (userMarks !== undefined && userMarks !== editor.marks) {
       editor.marks = userMarks
       editor.onChange()
+    } else if (userMarks !== undefined) {
+      // Just restore marks without triggering another onChange
+      editor.marks = userMarks
     }
   }
 
@@ -305,6 +310,8 @@ export function createAndroidInputManager({
       }
 
       updatePlaceholderVisibility()
+      // Schedule flush after storing diff to ensure onChange is triggered
+      scheduleFlush()
       return
     }
 
@@ -312,6 +319,8 @@ export function createAndroidInputManager({
     if (!merged) {
       pendingDiffs.splice(idx, 1)
       updatePlaceholderVisibility()
+      // Schedule flush after modifying diffs
+      scheduleFlush()
       return
     }
 
@@ -319,6 +328,8 @@ export function createAndroidInputManager({
       ...pendingDiffs[idx],
       diff: merged,
     }
+    // Schedule flush after updating diff
+    scheduleFlush()
   }

@changeset-bot
Copy link

changeset-bot bot commented Dec 9, 2025

🦋 Changeset detected

Latest commit: 7682d36

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@dylans dylans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please remove the comments for more obvious things? If you feel the comments are necessary I would prefer a variable for the conditional check that makes it clearer what condition is being met.

Also please add a changeset so that it will end up in a release.

Also please check our linting rules

} else if (userMarks !== undefined) {
editor.marks = userMarks
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing this might have been unintentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants