Fix Windows IME composition string being displayed incorrectly (#46545)

John Tur created 1 day ago

Previously, a new composition string was processed before a new
composition result. Thus, in the case that a keystroke creates a new
composition result and composition string, the processing of the new
result would overwrite the new string, even though they should both be
displayed. Processing them in the reverse order fixes this issue.
 
Fixes #42201

Additionally: while fixing this issue, I discovered that the Japanese
IME sometimes moves the cursor far away from the inserted text. WPF
works around this issue by only respecting the IME's requested cursor
position if it's adjacent to uncommitted text. So, we copy this
workaround.

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/windows/events.rs | 59 ++++++++++++++++++++---
1 file changed, 50 insertions(+), 9 deletions(-)

Detailed changes

crates/gpui/src/platform/windows/events.rs

@@ -632,22 +632,34 @@ impl WindowsWindowInner {
             })?;
             Some(0)
         } else {
+            if lparam & GCS_RESULTSTR.0 > 0 {
+                let comp_result = parse_ime_composition_string(ctx, GCS_RESULTSTR)?;
+                self.with_input_handler(|input_handler| {
+                    input_handler
+                        .replace_text_in_range(None, &String::from_utf16_lossy(&comp_result));
+                })?;
+            }
             if lparam & GCS_COMPSTR.0 > 0 {
                 let comp_string = parse_ime_composition_string(ctx, GCS_COMPSTR)?;
                 let caret_pos =
                     (!comp_string.is_empty() && lparam & GCS_CURSORPOS.0 > 0).then(|| {
-                        let pos = retrieve_composition_cursor_position(ctx);
+                        let cursor_pos = retrieve_composition_cursor_position(ctx);
+                        let pos = if should_use_ime_cursor_position(ctx, cursor_pos) {
+                            cursor_pos
+                        } else {
+                            comp_string.len()
+                        };
                         pos..pos
                     });
                 self.with_input_handler(|input_handler| {
-                    input_handler.replace_and_mark_text_in_range(None, &comp_string, caret_pos);
+                    input_handler.replace_and_mark_text_in_range(
+                        None,
+                        &String::from_utf16_lossy(&comp_string),
+                        caret_pos,
+                    );
                 })?;
             }
-            if lparam & GCS_RESULTSTR.0 > 0 {
-                let comp_result = parse_ime_composition_string(ctx, GCS_RESULTSTR)?;
-                self.with_input_handler(|input_handler| {
-                    input_handler.replace_text_in_range(None, &comp_result);
-                })?;
+            if lparam & (GCS_RESULTSTR.0 | GCS_COMPSTR.0) > 0 {
                 return Some(0);
             }
 
@@ -1450,7 +1462,7 @@ fn process_key(vkey: VIRTUAL_KEY, scan_code: u16) -> (Option<String>, bool) {
     )
 }
 
-fn parse_ime_composition_string(ctx: HIMC, comp_type: IME_COMPOSITION_STRING) -> Option<String> {
+fn parse_ime_composition_string(ctx: HIMC, comp_type: IME_COMPOSITION_STRING) -> Option<Vec<u16>> {
     unsafe {
         let string_len = ImmGetCompositionStringW(ctx, comp_type, None, 0);
         if string_len >= 0 {
@@ -1465,7 +1477,7 @@ fn parse_ime_composition_string(ctx: HIMC, comp_type: IME_COMPOSITION_STRING) ->
                 buffer.as_mut_ptr().cast::<u16>(),
                 string_len as usize / 2,
             );
-            Some(String::from_utf16_lossy(wstring))
+            Some(wstring.to_vec())
         } else {
             None
         }
@@ -1477,6 +1489,35 @@ fn retrieve_composition_cursor_position(ctx: HIMC) -> usize {
     unsafe { ImmGetCompositionStringW(ctx, GCS_CURSORPOS, None, 0) as usize }
 }
 
+fn should_use_ime_cursor_position(ctx: HIMC, cursor_pos: usize) -> bool {
+    let attrs_size = unsafe { ImmGetCompositionStringW(ctx, GCS_COMPATTR, None, 0) } as usize;
+    if attrs_size == 0 {
+        return false;
+    }
+
+    let mut attrs = vec![0u8; attrs_size];
+    let result = unsafe {
+        ImmGetCompositionStringW(
+            ctx,
+            GCS_COMPATTR,
+            Some(attrs.as_mut_ptr() as *mut _),
+            attrs_size as u32,
+        )
+    };
+    if result <= 0 {
+        return false;
+    }
+
+    // Keep the cursor adjacent to the inserted text by only using the suggested position
+    // if it's adjacent to unconverted text.
+    let at_cursor_is_input = cursor_pos < attrs.len() && attrs[cursor_pos] == (ATTR_INPUT as u8);
+    let before_cursor_is_input = cursor_pos > 0
+        && (cursor_pos - 1) < attrs.len()
+        && attrs[cursor_pos - 1] == (ATTR_INPUT as u8);
+
+    at_cursor_is_input || before_cursor_is_input
+}
+
 #[inline]
 fn is_virtual_key_pressed(vkey: VIRTUAL_KEY) -> bool {
     unsafe { GetKeyState(vkey.0 as i32) < 0 }