From 941bd6fa99b6f902f8331166998dcd5b1f0c9fd9 Mon Sep 17 00:00:00 2001 From: Cameron Mcloughlin Date: Thu, 27 Aug 2026 13:23:18 +0000 Subject: [PATCH] gpui: IME race condition bug (#63311) When an IME makes a selection, but doesn't modify the text, there was a race condition, where the GPUI->mirror sync could overwrite the `selectionchanged` event, causing IME sessions to be discarded. --- Release Notes: - N/A or Added/Fixed/Improved ... --- crates/gpui_web/src/events.rs | 6 ++++++ crates/gpui_web/src/ime_mirror.rs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/crates/gpui_web/src/events.rs b/crates/gpui_web/src/events.rs index 0c3cc8d4b9..73593a4e6a 100644 --- a/crates/gpui_web/src/events.rs +++ b/crates/gpui_web/src/events.rs @@ -825,6 +825,12 @@ impl WebWindowInner { }); if applied == Some(true) { this.ime_mirror.adopt_element_state(); + } else { + // No import is coming for this move; without a forced + // sync the mirror would keep deferring to it and show + // the IME a selection the app never adopted. + this.ime_mirror.reject_selection_import(); + this.schedule_ime_mirror_sync(); } }, )) diff --git a/crates/gpui_web/src/ime_mirror.rs b/crates/gpui_web/src/ime_mirror.rs index e519cfe2bb..7a4011aa4e 100644 --- a/crates/gpui_web/src/ime_mirror.rs +++ b/crates/gpui_web/src/ime_mirror.rs @@ -75,6 +75,12 @@ pub(crate) struct ImeMirror { /// between writes, and a mid-gesture barrage desynchronizes their word /// model. sync_scheduled: Cell, + /// Whether the `selectionchange` import saw an element selection move + /// it could not apply (the app selection changed underneath). Sync + /// normally defers to a pending import when the element's live + /// selection has moved; a rejected import means no import is coming, + /// so the next sync must reassert the app's state instead of waiting. + selection_import_rejected: Cell, } impl ImeMirror { @@ -111,6 +117,7 @@ impl ImeMirror { selection: Cell::new((0, 0)), window_hint: Cell::new(0), sync_scheduled: Cell::new(false), + selection_import_rejected: Cell::new(false), }; // Until an input handler asks otherwise, the element is an IME // conduit, not a form field: browser-side text assistance would @@ -230,6 +237,13 @@ impl ImeMirror { self.selection.set((selection_start, selection_end)); } + /// Records that an element selection move could not be imported, so + /// the next sync reasserts the app's state rather than deferring to an + /// import that is no longer coming. + pub(crate) fn reject_selection_import(&self) { + self.selection_import_rejected.set(true); + } + /// Schedules a coalesced sync of the mirror for the next task. /// /// Event handlers must not write to the mirror element mid-gesture: @@ -290,6 +304,23 @@ impl ImeMirror { return; } let mirror = &window.ime_mirror; + // A live element selection that differs from the stored baseline + // while the value still matches is an IME-driven selection move + // whose `selectionchange` import hasn't dispatched yet (the event + // is asynchronous, and this sync may run first). The element owns + // the selection until that import runs: writing now would clobber + // an in-progress gesture, e.g. Android's slide-on-backspace + // growing its selection. The import reconciles the two sides and + // schedules a fresh sync when it cannot adopt the move. + if !mirror.selection_import_rejected.replace(false) + && *mirror.text.borrow() == mirror.element.value() + { + let live_start = mirror.selection_start().unwrap_or(0); + let live_end = mirror.element_selection_end().unwrap_or(live_start); + if (live_start, live_end) != mirror.selection.get() { + return; + } + } let selection = window .with_input_handler(|handler| handler.selected_text_range(false)) .flatten();