On X11 query for XIM styles before creating IME

Fixes #2448.

CHANGELOG.md

@@ -26,6 +26,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 - On Wayland, fixed `Ime::Preedit` not being sent on IME reset.
 - Fixed unbound version specified for `raw-window-handle` leading to compilation failures.
 - Empty `Ime::Preedit` event will be sent before `Ime::Commit` to help clearing preedit.
+- On X11, fixed IME context picking by querying for supported styles beforehand.
 
 # 0.27.2 (2022-8-12)
 

src/platform_impl/linux/x11/ime/callbacks.rs

@@ -108,17 +108,17 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> {
     let mut new_contexts = HashMap::new();
     for (window, old_context) in (*inner).contexts.iter() {
         let spot = old_context.as_ref().map(|old_context| old_context.ic_spot);
-        let is_allowed = old_context
+        let style = old_context
             .as_ref()
-            .map(|old_context| old_context.is_allowed)
+            .map(|old_context| old_context.style)
             .unwrap_or_default();
         let new_context = {
             let result = ImeContext::new(
                 xconn,
                 new_im.im,
+                style,
                 *window,
                 spot,
-                is_allowed,
                 (*inner).event_sender.clone(),
             );
             if result.is_err() {
@@ -132,7 +132,7 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> {
     // If we've made it this far, everything succeeded.
     let _ = (*inner).destroy_all_contexts_if_necessary();
     let _ = (*inner).close_im_if_necessary();
-    (*inner).im = new_im.im;
+    (*inner).im = Some(new_im);
     (*inner).contexts = new_contexts;
     (*inner).is_destroyed = false;
     (*inner).is_fallback = is_fallback;

src/platform_impl/linux/x11/ime/context.rs

@@ -5,6 +5,7 @@ use std::{mem, ptr};
 
 use x11_dl::xlib::{XIMCallback, XIMPreeditCaretCallbackStruct, XIMPreeditDrawCallbackStruct};
 
+use crate::platform_impl::platform::x11::ime::input_method::{Style, XIMStyle};
 use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventSender};
 
 use super::{ffi, util, XConnection, XError};
@@ -191,9 +192,9 @@ struct ImeContextClientData {
 // still exists on the server. Since `ImeInner` has that awareness, destruction must be handled
 // through `ImeInner`.
 pub struct ImeContext {
-    pub(super) ic: ffi::XIC,
+    pub(crate) ic: ffi::XIC,
-    pub(super) ic_spot: ffi::XPoint,
+    pub(crate) ic_spot: ffi::XPoint,
-    pub(super) is_allowed: bool,
+    pub(crate) style: Style,
     // Since the data is passed shared between X11 XIM callbacks, but couldn't be direclty free from
     // there we keep the pointer to automatically deallocate it.
     _client_data: Box<ImeContextClientData>,
@@ -203,9 +204,9 @@ impl ImeContext {
     pub unsafe fn new(
         xconn: &Arc<XConnection>,
         im: ffi::XIM,
+        style: Style,
         window: ffi::Window,
         ic_spot: Option<ffi::XPoint>,
-        is_allowed: bool,
         event_sender: ImeEventSender,
     ) -> Result<Self, ImeContextCreationError> {
         let client_data = Box::into_raw(Box::new(ImeContextClientData {
@@ -215,12 +216,18 @@ impl ImeContext {
             cursor_pos: 0,
         }));
 
-        let ic = if is_allowed {
+        let ic = match style as _ {
-            ImeContext::create_ic(xconn, im, window, client_data as ffi::XPointer)
+            Style::Preedit(style) => ImeContext::create_preedit_ic(
-                .ok_or(ImeContextCreationError::Null)?
+                xconn,
-        } else {
+                im,
-            ImeContext::create_none_ic(xconn, im, window).ok_or(ImeContextCreationError::Null)?
+                style,
-        };
+                window,
+                client_data as ffi::XPointer,
+            ),
+            Style::Nothing(style) => ImeContext::create_nothing_ic(xconn, im, style, window),
+            Style::None(style) => ImeContext::create_none_ic(xconn, im, style, window),
+        }
+        .ok_or(ImeContextCreationError::Null)?;
 
         xconn
             .check_errors()
@@ -229,7 +236,7 @@ impl ImeContext {
         let mut context = ImeContext {
             ic,
             ic_spot: ffi::XPoint { x: 0, y: 0 },
-            is_allowed,
+            style,
             _client_data: Box::from_raw(client_data),
         };
 
@@ -244,12 +251,13 @@ impl ImeContext {
     unsafe fn create_none_ic(
         xconn: &Arc<XConnection>,
         im: ffi::XIM,
+        style: XIMStyle,
         window: ffi::Window,
     ) -> Option<ffi::XIC> {
         let ic = (xconn.xlib.XCreateIC)(
             im,
             ffi::XNInputStyle_0.as_ptr() as *const _,
-            ffi::XIMPreeditNone | ffi::XIMStatusNone,
+            style,
             ffi::XNClientWindow_0.as_ptr() as *const _,
             window,
             ptr::null_mut::<()>(),
@@ -258,9 +266,10 @@ impl ImeContext {
         (!ic.is_null()).then(|| ic)
     }
 
-    unsafe fn create_ic(
+    unsafe fn create_preedit_ic(
         xconn: &Arc<XConnection>,
         im: ffi::XIM,
+        style: XIMStyle,
         window: ffi::Window,
         client_data: ffi::XPointer,
     ) -> Option<ffi::XIC> {
@@ -282,32 +291,34 @@ impl ImeContext {
         )
         .expect("XVaCreateNestedList returned NULL");
 
-        let ic = {
+        let ic = (xconn.xlib.XCreateIC)(
-            let ic = (xconn.xlib.XCreateIC)(
+            im,
-                im,
+            ffi::XNInputStyle_0.as_ptr() as *const _,
-                ffi::XNInputStyle_0.as_ptr() as *const _,
+            style,
-                ffi::XIMPreeditCallbacks | ffi::XIMStatusNothing,
+            ffi::XNClientWindow_0.as_ptr() as *const _,
-                ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
-                window,
+            ffi::XNPreeditAttributes_0.as_ptr() as *const _,
-                ffi::XNPreeditAttributes_0.as_ptr() as *const _,
+            preedit_attr.ptr,
-                preedit_attr.ptr,
+            ptr::null_mut::<()>(),
-                ptr::null_mut::<()>(),
+        );
-            );
 
-            // If we've failed to create IC with preedit callbacks fallback to normal one.
+        (!ic.is_null()).then(|| ic)
-            if ic.is_null() {
+    }
-                (xconn.xlib.XCreateIC)(
+
-                    im,
+    unsafe fn create_nothing_ic(
-                    ffi::XNInputStyle_0.as_ptr() as *const _,
+        xconn: &Arc<XConnection>,
-                    ffi::XIMPreeditNothing | ffi::XIMStatusNothing,
+        im: ffi::XIM,
-                    ffi::XNClientWindow_0.as_ptr() as *const _,
+        style: XIMStyle,
-                    window,
+        window: ffi::Window,
-                    ptr::null_mut::<()>(),
+    ) -> Option<ffi::XIC> {
-                )
+        let ic = (xconn.xlib.XCreateIC)(
-            } else {
+            im,
-                ic
+            ffi::XNInputStyle_0.as_ptr() as *const _,
-            }
+            style,
-        };
+            ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
+            ptr::null_mut::<()>(),
+        );
 
         (!ic.is_null()).then(|| ic)
     }
@@ -326,13 +337,17 @@ impl ImeContext {
         xconn.check_errors()
     }
 
+    pub fn is_allowed(&self) -> bool {
+        !matches!(self.style, Style::None(_))
+    }
+
     // Set the spot for preedit text. Setting spot isn't working with libX11 when preedit callbacks
     // are being used. Certain IMEs do show selection window, but it's placed in bottom left of the
     // window and couldn't be changed.
     //
     // For me see: https://bugs.freedesktop.org/show_bug.cgi?id=1580.
     pub fn set_spot(&mut self, xconn: &Arc<XConnection>, x: c_short, y: c_short) {
-        if !self.is_allowed || self.ic_spot.x == x && self.ic_spot.y == y {
+        if !self.is_allowed() || self.ic_spot.x == x && self.ic_spot.y == y {
             return;
         }
 

src/platform_impl/linux/x11/ime/inner.rs

@@ -1,8 +1,11 @@
-use std::{collections::HashMap, mem, ptr, sync::Arc};
+use std::{collections::HashMap, mem, sync::Arc};
 
 use super::{ffi, XConnection, XError};
 
-use super::{context::ImeContext, input_method::PotentialInputMethods};
+use super::{
+    context::ImeContext,
+    input_method::{InputMethod, PotentialInputMethods},
+};
 use crate::platform_impl::platform::x11::ime::ImeEventSender;
 
 pub unsafe fn close_im(xconn: &Arc<XConnection>, im: ffi::XIM) -> Result<(), XError> {
@@ -17,8 +20,7 @@ pub unsafe fn destroy_ic(xconn: &Arc<XConnection>, ic: ffi::XIC) -> Result<(), X
 
 pub struct ImeInner {
     pub xconn: Arc<XConnection>,
-    // WARNING: this is initially null!
+    pub im: Option<InputMethod>,
-    pub im: ffi::XIM,
     pub potential_input_methods: PotentialInputMethods,
     pub contexts: HashMap<ffi::Window, Option<ImeContext>>,
     // WARNING: this is initially zeroed!
@@ -38,7 +40,7 @@ impl ImeInner {
     ) -> Self {
         ImeInner {
             xconn,
-            im: ptr::null_mut(),
+            im: None,
             potential_input_methods,
             contexts: HashMap::new(),
             destroy_callback: unsafe { mem::zeroed() },
@@ -49,8 +51,8 @@ impl ImeInner {
     }
 
     pub unsafe fn close_im_if_necessary(&self) -> Result<bool, XError> {
-        if !self.is_destroyed {
+        if !self.is_destroyed && self.im.is_some() {
-            close_im(&self.xconn, self.im).map(|_| true)
+            close_im(&self.xconn, self.im.as_ref().unwrap().im).map(|_| true)
         } else {
             Ok(false)
         }

src/platform_impl/linux/x11/ime/input_method.rs

@@ -2,7 +2,7 @@ use std::{
     env,
     ffi::{CStr, CString, IntoStringError},
     fmt,
-    os::raw::c_char,
+    os::raw::{c_char, c_ulong, c_ushort},
     ptr,
     sync::{Arc, Mutex},
 };
@@ -40,15 +40,97 @@ unsafe fn open_im(xconn: &Arc<XConnection>, locale_modifiers: &CStr) -> Option<f
 #[derive(Debug)]
 pub struct InputMethod {
     pub im: ffi::XIM,
+    pub preedit_style: Style,
+    pub none_style: Style,
     _name: String,
 }
 
 impl InputMethod {
-    fn new(im: ffi::XIM, name: String) -> Self {
+    fn new(xconn: &Arc<XConnection>, im: ffi::XIM, name: String) -> Option<Self> {
-        InputMethod { im, _name: name }
+        let mut styles: *mut XIMStyles = std::ptr::null_mut();
+
+        // Query the styles supported by the XIM.
+        unsafe {
+            if !(xconn.xlib.XGetIMValues)(
+                im,
+                ffi::XNQueryInputStyle_0.as_ptr() as *const _,
+                (&mut styles) as *mut _,
+                std::ptr::null_mut::<()>(),
+            )
+            .is_null()
+            {
+                return None;
+            }
+        }
+
+        let mut preedit_style = None;
+        let mut none_style = None;
+
+        unsafe {
+            std::slice::from_raw_parts((*styles).supported_styles, (*styles).count_styles as _)
+                .iter()
+                .for_each(|style| match *style {
+                    XIM_PREEDIT_STYLE => {
+                        preedit_style = Some(Style::Preedit(*style));
+                    }
+                    XIM_NOTHING_STYLE if preedit_style.is_none() => {
+                        preedit_style = Some(Style::Nothing(*style))
+                    }
+                    XIM_NONE_STYLE => none_style = Some(Style::None(*style)),
+                    _ => (),
+                });
+
+            (xconn.xlib.XFree)(styles.cast());
+        };
+
+        if preedit_style.is_none() && none_style.is_none() {
+            return None;
+        }
+
+        let preedit_style = preedit_style.unwrap_or_else(|| none_style.unwrap());
+        let none_style = none_style.unwrap_or(preedit_style);
+
+        Some(InputMethod {
+            im,
+            _name: name,
+            preedit_style,
+            none_style,
+        })
     }
 }
 
+const XIM_PREEDIT_STYLE: XIMStyle = (ffi::XIMPreeditCallbacks | ffi::XIMStatusNothing) as XIMStyle;
+const XIM_NOTHING_STYLE: XIMStyle = (ffi::XIMPreeditNothing | ffi::XIMStatusNothing) as XIMStyle;
+const XIM_NONE_STYLE: XIMStyle = (ffi::XIMPreeditNone | ffi::XIMStatusNone) as XIMStyle;
+
+/// Style of the IME context.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Style {
+    /// Preedit callbacks.
+    Preedit(XIMStyle),
+
+    /// Nothing.
+    Nothing(XIMStyle),
+
+    /// No IME.
+    None(XIMStyle),
+}
+
+impl Default for Style {
+    fn default() -> Self {
+        Style::None(XIM_NONE_STYLE)
+    }
+}
+
+#[repr(C)]
+#[derive(Debug)]
+struct XIMStyles {
+    count_styles: c_ushort,
+    supported_styles: *const XIMStyle,
+}
+
+pub(crate) type XIMStyle = c_ulong;
+
 #[derive(Debug)]
 pub enum InputMethodResult {
     /// Input method used locale modifier from `XMODIFIERS` environment variable.
@@ -174,7 +256,7 @@ impl PotentialInputMethod {
     pub fn open_im(&mut self, xconn: &Arc<XConnection>) -> Option<InputMethod> {
         let im = unsafe { open_im(xconn, &self.name.c_string) };
         self.successful = Some(im.is_some());
-        im.map(|im| InputMethod::new(im, self.name.string.clone()))
+        im.and_then(|im| InputMethod::new(xconn, im, self.name.string.clone()))
     }
 }
 

src/platform_impl/linux/x11/ime/mod.rs

@@ -17,8 +17,9 @@ use self::{
     callbacks::*,
     context::ImeContext,
     inner::{close_im, ImeInner},
-    input_method::PotentialInputMethods,
+    input_method::{PotentialInputMethods, Style},
 };
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum ImeEvent {
@@ -87,7 +88,6 @@ impl Ime {
 
         let is_fallback = input_method.is_fallback();
         if let Some(input_method) = input_method.ok() {
-            inner.im = input_method.im;
             inner.is_fallback = is_fallback;
             unsafe {
                 let result = set_destroy_callback(&xconn, input_method.im, &*inner)
@@ -97,6 +97,7 @@ impl Ime {
                 }
                 result?;
             }
+            inner.im = Some(input_method);
             Ok(Ime { xconn, inner })
         } else {
             Err(ImeCreationError::OpenFailure(inner.potential_input_methods))
@@ -120,37 +121,35 @@ impl Ime {
             // Create empty entry in map, so that when IME is rebuilt, this window has a context.
             None
         } else {
+            let im = self.inner.im.as_ref().unwrap();
+            let style = if with_preedit {
+                im.preedit_style
+            } else {
+                im.none_style
+            };
+
             let context = unsafe {
                 ImeContext::new(
                     &self.inner.xconn,
-                    self.inner.im,
+                    im.im,
+                    style,
                     window,
                     None,
-                    with_preedit,
                     self.inner.event_sender.clone(),
-                )
+                )?
-                .or_else(|_| {
+            };
-                    debug!(
-                        "failed to create an IME context {} preedit support",
-                        if with_preedit { "with" } else { "without" }
-                    );
-                    ImeContext::new(
-                        &self.inner.xconn,
-                        self.inner.im,
-                        window,
-                        None,
-                        !with_preedit,
-                        self.inner.event_sender.clone(),
-                    )
-                })
-            }?;
 
             // Check the state on the context, since it could fail to enable or disable preedit.
-            let event = if context.is_allowed {
+            let event = if matches!(style, Style::None(_)) {
-                ImeEvent::Enabled
+                if with_preedit {
-            } else {
+                    debug!("failed to create IME context with preedit support.")
-                // There's no IME without preedit.
+                }
                 ImeEvent::Disabled
+            } else {
+                if !with_preedit {
+                    debug!("failed to create IME context without preedit support.")
+                }
+                ImeEvent::Enabled
             };
 
             self.inner
@@ -160,6 +159,7 @@ impl Ime {
 
             Some(context)
         };
+
         self.inner.contexts.insert(window, context);
         Ok(!self.is_destroyed())
     }
@@ -223,7 +223,7 @@ impl Ime {
         }
 
         if let Some(&mut Some(ref mut context)) = self.inner.contexts.get_mut(&window) {
-            if allowed == context.is_allowed {
+            if allowed == context.is_allowed() {
                 return;
             }
         }