x11: Don't panic when using dead keys (#432)

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Unreleased
 
 - Added subclass to macos windows so they can be made resizable even with no decorations.
+- Dead keys now work properly on X11, no longer resulting in a panic.
 
 # Version 0.11.3 (2018-03-28)
 

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,8 +199,14 @@ impl EventsLoop {
     {
         let xlib = &self.display.xlib;
 
-        // Handle dead keys and other input method funtimes
+        // XFilterEvent tells us when an event has been discarded by the input method.
-        if ffi::True == unsafe { (self.display.xlib.XFilterEvent)(xev, { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window }) } {
+        // Specifically, this involves all of the KeyPress events in compose/pre-edit sequences,
+        // along with an extra copy of the KeyRelease events. This also prevents backspace and
+        // arrow keys from being detected twice.
+        if ffi::True == unsafe { (self.display.xlib.XFilterEvent)(
+            xev,
+            { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window }
+        ) } {
             return;
         }
 
@@ -414,16 +420,15 @@ 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;
+                // Note that in compose/pre-edit sequences, this will always be Released.
-                if xev.get_type() == ffi::KeyPress {
+                let state = if xev.get_type() == ffi::KeyPress {
-                    state = Pressed;
+                    Pressed
                 } else {
-                    state = Released;
+                    Released
-                }
+                };
 
                 let xkev: &mut ffi::XKeyEvent = xev.as_mut();
 
@@ -439,55 +444,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);
+                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 {
-                     // Typical virtual core keyboard ID. xinput2 needs to be used to get a reliable value.
+                        // 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: state,
+                            state,
                             scancode: xkev.keycode - 8,
-                        virtual_keycode: vkey,
+                            virtual_keycode,
                             modifiers,
                         },
                     }});
+                }
 
                 if state == Pressed {
-                    let written = unsafe {
+                    let written = {
-                        use std::str;
+                        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()
+}