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 ...
This commit is contained in:
1 parent
09440c85cb
commit
941bd6fa99
2 files changed
+37
No files matched your search
@@ -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();
|
||||
}
|
||||
},
|
||||
))
|
||||
|
||||
@@ -75,6 +75,12 @@ pub(crate) struct ImeMirror {
|
||||
/// between writes, and a mid-gesture barrage desynchronizes their word
|
||||
/// model.
|
||||
sync_scheduled: Cell<bool>,
|
||||
/// 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<bool>,
|
||||
}
|
||||
|
||||
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();
|
||||
|
||||
Reference in new issue
Block a user