x11: Don't panic when using dead keys

CHANGELOG.md

@@ -1,12 +1,12 @@
 # Unreleased
-- Added `set_min_dimensions` and `set_max_dimensions` methods to `Window`, and implemented on Windows, X11, Wayland, and OSX.
 
+- Added `set_min_dimensions` and `set_max_dimensions` methods to `Window`, and implemented on Windows, X11, Wayland, and OSX.
 - On X11, dropping a `Window` actually closes it now, and clicking the window's × button (or otherwise having the WM signal to close it) will result in the window closing.
 - Added `WindowBuilderExt` methods for macos: `with_titlebar_transparent`,
   `with_title_hidden`, `with_titlebar_buttons_hidden`,
   `with_fullsize_content_view`.
-
 - Mapped X11 numpad keycodes (arrows, Home, End, PageUp, PageDown, Insert and Delete) to corresponding virtual keycodes
+- Dead keys now work properly on X11, no longer resulting in a panic.
 
 # Version 0.11.2 (2018-03-06)
 

src/platform/linux/x11/mod.rs

@@ -16,7 +16,7 @@ use std::sync::{Arc, Mutex, Weak};
 use std::sync::atomic::{self, AtomicBool};
 use std::collections::HashMap;
 use std::ffi::CStr;
-use std::os::raw::{c_char, c_int, c_long, c_uchar, c_ulong};
+use std::os::raw::{c_char, c_int, c_long, c_uchar, c_uint, c_ulong};
 
 use libc;
 
@@ -199,9 +199,26 @@ impl EventsLoop {
     {
         let xlib = &self.display.xlib;
 
-        // Handle dead keys and other input method funtimes
-        if ffi::True == unsafe { (self.display.xlib.XFilterEvent)(xev, { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window }) } {
-            return;
+        // XFilterEvent tells us when an event has been discarded by the input method.
+        // Specifically, this involves all of the KeyPress events in compose/pre-edit sequences,
+        // along with an extra copy of the KeyRelease events.
+        let filtered = ffi::True == unsafe { (self.display.xlib.XFilterEvent)(
+            xev,
+            { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window }
+        ) };
+
+        if filtered {
+            // If we directly follow the recommendation of XFilterEvent, we'll be getting
+            // KeyRelease events without their corresponding KeyPress events. This condition
+            // ensures that we only ignore the duplicate KeyRelease events.
+            //
+            // While we still want to propagate these KeyPress events, we have to remember not to
+            // emit events for the characters received from them. The `filtered` variable is
+            // created for that exact purpose, allowing us to make that check in the KeyPress
+            // handler.
+            if xev.get_type() == ffi::KeyRelease {
+                return;
+            }
         }
 
         match xev.get_type() {
@@ -414,16 +431,14 @@ impl EventsLoop {
                 callback(Event::WindowEvent { window_id, event: WindowEvent::Refresh });
             }
 
-            // FIXME: Use XInput2 + libxkbcommon for keyboard input!
             ffi::KeyPress | ffi::KeyRelease => {
                 use events::ElementState::{Pressed, Released};
 
-                let state;
-                if xev.get_type() == ffi::KeyPress {
-                    state = Pressed;
+                let state = if xev.get_type() == ffi::KeyPress {
+                    Pressed
                 } else {
-                    state = Released;
-                }
+                    Released
+                };
 
                 let xkev: &mut ffi::XKeyEvent = xev.as_mut();
 
@@ -439,55 +454,50 @@ impl EventsLoop {
 
                 let keysym = unsafe {
                     let mut keysym = 0;
-                    (self.display.xlib.XLookupString)(xkev, ptr::null_mut(), 0, &mut keysym, ptr::null_mut());
+                    (self.display.xlib.XLookupString)(
+                        xkev,
+                        ptr::null_mut(),
+                        0,
+                        &mut keysym,
+                        ptr::null_mut(),
+                    );
                     keysym
                 };
 
-                let vkey = events::keysym_to_element(keysym as libc::c_uint);
-
-                callback(Event::WindowEvent { window_id, event: WindowEvent::KeyboardInput {
-                     // Typical virtual core keyboard ID. xinput2 needs to be used to get a reliable value.
-                    device_id: mkdid(3),
-                    input: KeyboardInput {
-                        state: state,
-                        scancode: xkev.keycode - 8,
-                        virtual_keycode: vkey,
-                        modifiers,
-                    },
-                }});
+                let virtual_keycode = events::keysym_to_element(keysym as c_uint);
+
+                // When a compose sequence or IME pre-edit is finished, it ends in a KeyPress with
+                // a keycode of 0.
+                if xkev.keycode != 0 {
+                    callback(Event::WindowEvent { window_id, event: WindowEvent::KeyboardInput {
+                        // Standard virtual core keyboard ID. XInput2 needs to be used to get a
+                        // reliable value, though this should only be an issue under multiseat
+                        // configurations.
+                        device_id: mkdid(3),
+                        input: KeyboardInput {
+                            state,
+                            scancode: xkev.keycode - 8,
+                            virtual_keycode,
+                            modifiers,
+                        },
+                    }});
+                }
 
-                if state == Pressed {
-                    let written = unsafe {
-                        use std::str;
+                if state == Pressed && !filtered {
+                    let written = {
+                        let windows = self.windows.lock().unwrap();
 
-                        const INIT_BUFF_SIZE: usize = 16;
-                        let mut windows = self.windows.lock().unwrap();
                         let window_data = {
-                            if let Some(window_data) = windows.get_mut(&WindowId(window)) {
+                            if let Some(window_data) = windows.get(&WindowId(window)) {
                                 window_data
                             } else {
                                 return;
                             }
                         };
-                        /* buffer allocated on heap instead of stack, due to the possible
-                         * reallocation */
-                        let mut buffer: Vec<u8> = vec![mem::uninitialized(); INIT_BUFF_SIZE];
-                        let mut keysym: ffi::KeySym = 0;
-                        let mut status: ffi::Status = 0;
-                        let mut count = (self.display.xlib.Xutf8LookupString)(window_data.ic, xkev,
-                                                                          mem::transmute(buffer.as_mut_ptr()),
-                                                                          buffer.len() as libc::c_int,
-                                                                          &mut keysym, &mut status);
-                        /* buffer overflowed, dynamically reallocate */
-                        if status == ffi::XBufferOverflow {
-                            buffer = vec![mem::uninitialized(); count as usize];
-                            count = (self.display.xlib.Xutf8LookupString)(window_data.ic, xkev,
-                                                                          mem::transmute(buffer.as_mut_ptr()),
-                                                                          buffer.len() as libc::c_int,
-                                                                          &mut keysym, &mut status);
-                        }
 
-                        str::from_utf8(&buffer[..count as usize]).unwrap_or("").to_string()
+                        unsafe {
+                            util::lookup_utf8(&self.display, window_data.ic, xkev)
+                        }
                     };
 
                     for chr in written.chars() {

src/platform/linux/x11/util.rs

@@ -1,5 +1,6 @@
 use std::mem;
 use std::ptr;
+use std::str;
 use std::sync::Arc;
 use std::ops::{Deref, DerefMut};
 use std::os::raw::{c_char, c_double, c_int, c_long, c_short, c_uchar, c_uint, c_ulong};
@@ -247,3 +248,53 @@ pub unsafe fn query_pointer(
         relative_to_window,
     })
 }
+
+unsafe fn lookup_utf8_inner(
+    xconn: &Arc<XConnection>,
+    ic: ffi::XIC,
+    key_event: &mut ffi::XKeyEvent,
+    buffer: &mut [u8],
+) -> (ffi::KeySym, ffi::Status, c_int) {
+    let mut keysym: ffi::KeySym = 0;
+    let mut status: ffi::Status = 0;
+    let count = (xconn.xlib.Xutf8LookupString)(
+        ic,
+        key_event,
+        buffer.as_mut_ptr() as *mut c_char,
+        buffer.len() as c_int,
+        &mut keysym,
+        &mut status,
+    );
+    (keysym, status, count)
+}
+
+pub unsafe fn lookup_utf8(
+    xconn: &Arc<XConnection>,
+    ic: ffi::XIC,
+    key_event: &mut ffi::XKeyEvent,
+) -> String {
+    const INIT_BUFF_SIZE: usize = 16;
+
+    // Buffer allocated on heap instead of stack, due to the possible reallocation
+    let mut buffer: Vec<u8> = vec![mem::uninitialized(); INIT_BUFF_SIZE];
+    let (_, status, mut count) = lookup_utf8_inner(
+        xconn,
+        ic,
+        key_event,
+        &mut buffer,
+    );
+
+    // Buffer overflowed, dynamically reallocate
+    if status == ffi::XBufferOverflow {
+        buffer = vec![mem::uninitialized(); count as usize];
+        let (_, _, new_count) = lookup_utf8_inner(
+            xconn,
+            ic,
+            key_event,
+            &mut buffer,
+        );
+        count = new_count;
+    }
+
+    str::from_utf8(&buffer[..count as usize]).unwrap_or("").to_string()
+}