mirror of
https://github.com/emilk/egui.git
synced 2026-06-26 22:53:14 -04:00
Fix backspacing leaving last character in IME prediction not removed on macOS native and Safari (#7810)
<!-- Please read the "Making a PR" section of [`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/main/CONTRIBUTING.md) before opening a Pull Request! * Keep your PR:s small and focused. * The PR title is what ends up in the changelog, so make it descriptive! * If applicable, add a screenshot or gif. * If it is a non-trivial addition, consider adding a demo for it to `egui_demo_lib`, or a new example. * Do NOT open PR:s from your `master` branch, as that makes it hard for maintainers to test and add commits to your PR. * Remember to run `cargo fmt` and `cargo clippy`. * Open the PR as a draft until you have self-reviewed it and run `./scripts/check.sh`. * When you have addressed a PR comment, mark it as resolved. Please be patient! I will review your PR, but my time is limited! --> * Closes N/A * [x] I have followed the instructions in the PR template ## Before the fix | Platform | Screenshot | | - | - | | macOS native |  | | Safari |  | ## After the fix | Platform | Screenshot | | - | - | | macOS native |  | | Safari |  | (The font used in the screenshots is [GNU Unifont](https://unifoundry.com/unifont/index.html), licensed under [OFL-1.1.txt](https://unifoundry.com/OFL-1.1.txt).)
This commit is contained in:
@@ -352,45 +352,7 @@ impl State {
|
||||
}
|
||||
|
||||
WindowEvent::Ime(ime) => {
|
||||
// on Mac even Cmd-C is pressed during ime, a `c` is pushed to Preedit.
|
||||
// So no need to check is_mac_cmd.
|
||||
//
|
||||
// How winit produce `Ime::Enabled` and `Ime::Disabled` differs in MacOS
|
||||
// and Windows.
|
||||
//
|
||||
// - On Windows, before and after each Commit will produce an Enable/Disabled
|
||||
// event.
|
||||
// - On MacOS, only when user explicit enable/disable ime. No Disabled
|
||||
// after Commit.
|
||||
//
|
||||
// We use input_method_editor_started to manually insert CompositionStart
|
||||
// between Commits.
|
||||
match ime {
|
||||
winit::event::Ime::Enabled => {
|
||||
if cfg!(target_os = "linux") {
|
||||
// This event means different things in X11 and Wayland, but we can just
|
||||
// ignore it and enable IME on the preedit event.
|
||||
// See <https://github.com/rust-windowing/winit/issues/2498>
|
||||
} else {
|
||||
self.ime_event_enable();
|
||||
}
|
||||
}
|
||||
winit::event::Ime::Preedit(text, Some(_cursor)) => {
|
||||
self.ime_event_enable();
|
||||
self.egui_input
|
||||
.events
|
||||
.push(egui::Event::Ime(egui::ImeEvent::Preedit(text.clone())));
|
||||
}
|
||||
winit::event::Ime::Commit(text) => {
|
||||
self.egui_input
|
||||
.events
|
||||
.push(egui::Event::Ime(egui::ImeEvent::Commit(text.clone())));
|
||||
self.ime_event_disable();
|
||||
}
|
||||
winit::event::Ime::Disabled | winit::event::Ime::Preedit(_, None) => {
|
||||
self.ime_event_disable();
|
||||
}
|
||||
}
|
||||
self.on_ime(ime);
|
||||
|
||||
EventResponse {
|
||||
repaint: true,
|
||||
@@ -564,6 +526,104 @@ impl State {
|
||||
}
|
||||
}
|
||||
|
||||
/// ## NOTE
|
||||
///
|
||||
/// on Mac even Cmd-C is pressed during ime, a `c` is pushed to Preedit.
|
||||
/// So no need to check `is_mac_cmd`.
|
||||
///
|
||||
/// ### How events are emitted by [`winit`] across different setups in various situations
|
||||
///
|
||||
/// This is done by uncommenting the code block at the top of this method
|
||||
/// and checking console outputs.
|
||||
///
|
||||
/// winit version: 0.30.12.
|
||||
///
|
||||
/// #### Setups
|
||||
///
|
||||
/// - `a-macos15-apple_shuangpin`: macOS 15.7.3 `aarch64`, IME: builtin Chinese Shuangpin - Simplified. (Demo app shows: renderer: `wgpu`, backend: `Metal`.)
|
||||
/// - `b-debian13_gnome48_wayland-fcitx5_shuangpin`: Debian 13 `aarch64`, Gnome 48, Wayland, IME: Fcitx5 with fcitx5-chinese-addons's Shuangpin. (Demo app shows: renderer: `wgpu`, backend: `Gl`.)
|
||||
/// - `c-windows11-ms_pinyin`: Windows11 23H2 `x86_64`, IME: builtin Microsoft Pinyin. (Demo app shows: renderer: `wgpu`, backend: `Vulkan` & `Dx12`, others: `Dx12` & `Gl`.)
|
||||
///
|
||||
/// #### Situation: pressed space to select the first candidate "测试"
|
||||
///
|
||||
/// | Setup | Events in Order |
|
||||
/// | ------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- |
|
||||
/// | a-macos15-apple_shuangpin | `Predict("", None)` -> `Commit("测试")` |
|
||||
/// | b-debian13_gnome48_wayland-fcitx5_shuangpin | `Predict("", None)` -> `Commit("测试")` -> `Predict("", Some(0, 0))` -> `Predict("", None)` (duplicate until `TextEdit` blurred) |
|
||||
/// | c-windows11-ms_pinyin | `Predict("测试", Some(…))` -> `Predict("", None)` -> `Commit("测试")` -> `Disabled` |
|
||||
///
|
||||
/// #### Situation: pressed backspace to delete the last character in the prediction
|
||||
///
|
||||
/// | Setup | Events in Order |
|
||||
/// | a-macos15-apple_shuangpin | `Predict("", None)` |
|
||||
/// | b-debian13_gnome48_wayland-fcitx5_shuangpin | `Predict("", Some(0, 0))` -> `Predict("", None)` (duplicate until `TextEdit` blurred) |
|
||||
/// | c-windows11-ms_pinyin | `Predict("", Some(0, 0))` -> `Predict("", None)` -> `Commit("")` -> `Disabled` |
|
||||
///
|
||||
/// #### Situation: clicked somewhere else while there is an active composition with the prediction "ce"
|
||||
///
|
||||
/// | Setup | Events in Order |
|
||||
/// | ------------------------------------------- | ------------------------------------------------------------------------------------------------- |
|
||||
/// | a-macos15-apple_shuangpin | nothing emitted |
|
||||
/// | b-debian13_gnome48_wayland-fcitx5_shuangpin | `Predict("", Some(0, 0))` (duplicate) -> `Predict("", None)` (duplicate until `TextEdit` blurred) |
|
||||
/// | c-windows11-ms_pinyin | nothing emitted |
|
||||
fn on_ime(&mut self, ime: &winit::event::Ime) {
|
||||
// // code for inspecting ime events emitted by winit:
|
||||
// {
|
||||
// static LAST_IME: std::sync::Mutex<Option<winit::event::Ime>> =
|
||||
// std::sync::Mutex::new(None);
|
||||
// static IS_LAST_DUPLICATE: std::sync::atomic::AtomicBool =
|
||||
// std::sync::atomic::AtomicBool::new(false);
|
||||
// let mut last_ime_guard = LAST_IME.lock().unwrap();
|
||||
// if { last_ime_guard.as_ref().cloned() }.as_ref() != Some(ime) {
|
||||
// println!("IME={ime:?}");
|
||||
// *last_ime_guard = Some(ime.clone());
|
||||
// IS_LAST_DUPLICATE.store(false, std::sync::atomic::Ordering::Relaxed);
|
||||
// } else if !IS_LAST_DUPLICATE.load(std::sync::atomic::Ordering::Relaxed) {
|
||||
// println!("IME=(duplicate)");
|
||||
// IS_LAST_DUPLICATE.store(true, std::sync::atomic::Ordering::Relaxed);
|
||||
// }
|
||||
// }
|
||||
|
||||
match ime {
|
||||
winit::event::Ime::Enabled => {
|
||||
if cfg!(target_os = "linux") {
|
||||
// This event means different things in X11 and Wayland, but we can just
|
||||
// ignore it and enable IME on the preedit event.
|
||||
// See <https://github.com/rust-windowing/winit/issues/2498>
|
||||
} else {
|
||||
self.ime_event_enable();
|
||||
}
|
||||
}
|
||||
winit::event::Ime::Preedit(text, Some(_cursor)) => {
|
||||
self.ime_event_enable();
|
||||
self.egui_input
|
||||
.events
|
||||
.push(egui::Event::Ime(egui::ImeEvent::Preedit(text.clone())));
|
||||
}
|
||||
winit::event::Ime::Commit(text) => {
|
||||
self.egui_input
|
||||
.events
|
||||
.push(egui::Event::Ime(egui::ImeEvent::Commit(text.clone())));
|
||||
self.ime_event_disable();
|
||||
}
|
||||
winit::event::Ime::Disabled => {
|
||||
self.ime_event_disable();
|
||||
}
|
||||
winit::event::Ime::Preedit(_, None) => {
|
||||
// we need to emit this on macOS, since winit doesn't emit
|
||||
// `Predict("", Some(0, 0))` before this event on macOS when the
|
||||
// user deletes the last character in the prediction with the
|
||||
// backspace key. Without this, only `egui::ImeEvent::Disabled`
|
||||
// is emitted here, leading to the last character being left in
|
||||
// TextEdit in such situation.
|
||||
self.egui_input
|
||||
.events
|
||||
.push(egui::Event::Ime(egui::ImeEvent::Preedit(String::new())));
|
||||
self.ime_event_disable();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn ime_event_enable(&mut self) {
|
||||
if !self.has_sent_ime_enabled {
|
||||
self.egui_input
|
||||
|
||||
@@ -1065,51 +1065,73 @@ fn events(
|
||||
..
|
||||
} => check_for_mutating_key_press(os, &cursor_range, text, galley, modifiers, *key),
|
||||
|
||||
Event::Ime(ime_event) => match ime_event {
|
||||
ImeEvent::Enabled => {
|
||||
state.ime_enabled = true;
|
||||
state.ime_cursor_range = cursor_range;
|
||||
None
|
||||
Event::Ime(ime_event) => {
|
||||
/// Empty prediction can be produced with [`ImeEvent::Preedit`]
|
||||
/// or [`ImeEvent::Commit`] when user press backspace or escape
|
||||
/// during IME, so this function should be called in both cases
|
||||
/// to clear current text.
|
||||
///
|
||||
/// Example platforms where only `ImeEvent::Preedit("")` of
|
||||
/// those two events is emitted when the last character in the
|
||||
/// prediction is deleted:
|
||||
/// - macOS 15.7.3.
|
||||
/// - Debian13 with gnome48 and wayland.
|
||||
///
|
||||
/// An example platform where only `ImeEvent::Commit("")` of
|
||||
/// those two events is emitted when the last character in the
|
||||
/// prediction is deleted:
|
||||
/// - Safari 26.2 (on macOS 15.7.3).
|
||||
fn clear_prediction(
|
||||
text: &mut dyn TextBuffer,
|
||||
cursor_range: &CCursorRange,
|
||||
) -> CCursor {
|
||||
text.delete_selected(cursor_range)
|
||||
}
|
||||
ImeEvent::Preedit(text_mark) => {
|
||||
if text_mark == "\n" || text_mark == "\r" {
|
||||
None
|
||||
} else {
|
||||
// Empty prediction can be produced when user press backspace
|
||||
// or escape during IME, so we clear current text.
|
||||
let mut ccursor = text.delete_selected(&cursor_range);
|
||||
let start_cursor = ccursor;
|
||||
if !text_mark.is_empty() {
|
||||
text.insert_text_at(&mut ccursor, text_mark, char_limit);
|
||||
}
|
||||
state.ime_cursor_range = cursor_range;
|
||||
Some(CCursorRange::two(start_cursor, ccursor))
|
||||
}
|
||||
}
|
||||
ImeEvent::Commit(prediction) => {
|
||||
if prediction == "\n" || prediction == "\r" {
|
||||
None
|
||||
} else {
|
||||
state.ime_enabled = false;
|
||||
|
||||
if !prediction.is_empty()
|
||||
&& cursor_range.secondary.index
|
||||
== state.ime_cursor_range.secondary.index
|
||||
{
|
||||
let mut ccursor = text.delete_selected(&cursor_range);
|
||||
text.insert_text_at(&mut ccursor, prediction, char_limit);
|
||||
Some(CCursorRange::one(ccursor))
|
||||
match ime_event {
|
||||
ImeEvent::Enabled => {
|
||||
state.ime_enabled = true;
|
||||
state.ime_cursor_range = cursor_range;
|
||||
None
|
||||
}
|
||||
ImeEvent::Preedit(text_mark) => {
|
||||
if text_mark == "\n" || text_mark == "\r" {
|
||||
None
|
||||
} else {
|
||||
let ccursor = cursor_range.primary;
|
||||
let mut ccursor = clear_prediction(text, &cursor_range);
|
||||
|
||||
let start_cursor = ccursor;
|
||||
if !text_mark.is_empty() {
|
||||
text.insert_text_at(&mut ccursor, text_mark, char_limit);
|
||||
}
|
||||
state.ime_cursor_range = cursor_range;
|
||||
Some(CCursorRange::two(start_cursor, ccursor))
|
||||
}
|
||||
}
|
||||
ImeEvent::Commit(prediction) => {
|
||||
if prediction == "\n" || prediction == "\r" {
|
||||
None
|
||||
} else {
|
||||
state.ime_enabled = false;
|
||||
|
||||
let mut ccursor = clear_prediction(text, &cursor_range);
|
||||
|
||||
if !prediction.is_empty()
|
||||
&& cursor_range.secondary.index
|
||||
== state.ime_cursor_range.secondary.index
|
||||
{
|
||||
text.insert_text_at(&mut ccursor, prediction, char_limit);
|
||||
}
|
||||
|
||||
Some(CCursorRange::one(ccursor))
|
||||
}
|
||||
}
|
||||
ImeEvent::Disabled => {
|
||||
state.ime_enabled = false;
|
||||
None
|
||||
}
|
||||
}
|
||||
ImeEvent::Disabled => {
|
||||
state.ime_enabled = false;
|
||||
None
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
_ => None,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user