From a6998af997b4d78aa31393fe5e1ade9c88490adc Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Mon, 3 Feb 2025 20:43:43 +0300 Subject: [PATCH] x11: fix crash with uim Let's just not forward events to the IME once the user requested that it should be disabled, though, still try to change its state explicitly. Fixes #4082. --- src/changelog/unreleased.md | 1 + src/platform_impl/linux/x11/ime/callbacks.rs | 8 ++----- src/platform_impl/linux/x11/ime/context.rs | 22 ++++++++++------- .../linux/x11/ime/input_method.rs | 4 +--- src/platform_impl/linux/x11/ime/mod.rs | 24 ++++--------------- 5 files changed, 22 insertions(+), 37 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 79558b72..91860cc4 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -227,3 +227,4 @@ changelog entry. - On macOS, fixed redundant `SurfaceResized` event at window creation. - On Windows, fixed the event loop not waking on accessibility requests. - On X11, fixed cursor grab mode state tracking on error. +- On X11, fixed crash with uim diff --git a/src/platform_impl/linux/x11/ime/callbacks.rs b/src/platform_impl/linux/x11/ime/callbacks.rs index c4540b2d..16737aea 100644 --- a/src/platform_impl/linux/x11/ime/callbacks.rs +++ b/src/platform_impl/linux/x11/ime/callbacks.rs @@ -122,19 +122,15 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> { let is_allowed = old_context.as_ref().map(|old_context| old_context.is_allowed()).unwrap_or_default(); - // We can't use the style from the old context here, since it may change on reload, so - // pick style from the new XIM based on the old state. - let style = if is_allowed { new_im.preedit_style } else { new_im.none_style }; - let new_context = { let result = unsafe { ImeContext::new( xconn, - new_im.im, - style, + &new_im, *window, spot, (*inner).event_sender.clone(), + is_allowed, ) }; if result.is_err() { diff --git a/src/platform_impl/linux/x11/ime/context.rs b/src/platform_impl/linux/x11/ime/context.rs index bb1ef642..b981c999 100644 --- a/src/platform_impl/linux/x11/ime/context.rs +++ b/src/platform_impl/linux/x11/ime/context.rs @@ -7,7 +7,7 @@ use std::{fmt, mem, ptr}; use x11_dl::xlib::{XIMCallback, XIMPreeditCaretCallbackStruct, XIMPreeditDrawCallbackStruct}; use super::{ffi, util, XConnection, XError}; -use crate::platform_impl::platform::x11::ime::input_method::{Style, XIMStyle}; +use crate::platform_impl::platform::x11::ime::input_method::{InputMethod, Style, XIMStyle}; use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventSender}; /// IME creation error. @@ -197,7 +197,7 @@ struct ImeContextClientData { pub struct ImeContext { pub(crate) ic: ffi::XIC, pub(crate) ic_spot: ffi::XPoint, - pub(crate) style: Style, + pub(crate) allowed: bool, // Since the data is passed shared between X11 XIM callbacks, but couldn't be directly free // from there we keep the pointer to automatically deallocate it. _client_data: Box, @@ -206,11 +206,11 @@ pub struct ImeContext { impl ImeContext { pub(crate) unsafe fn new( xconn: &Arc, - im: ffi::XIM, - style: Style, + im: &InputMethod, window: ffi::Window, ic_spot: Option, event_sender: ImeEventSender, + allowed: bool, ) -> Result { let client_data = Box::into_raw(Box::new(ImeContextClientData { window, @@ -219,20 +219,24 @@ impl ImeContext { cursor_pos: 0, })); + let style = if allowed { im.preedit_style } else { im.none_style }; + let ic = match style as _ { Style::Preedit(style) => unsafe { ImeContext::create_preedit_ic( xconn, - im, + im.im, style, window, client_data as ffi::XPointer, ) }, Style::Nothing(style) => unsafe { - ImeContext::create_nothing_ic(xconn, im, style, window) + ImeContext::create_nothing_ic(xconn, im.im, style, window) + }, + Style::None(style) => unsafe { + ImeContext::create_none_ic(xconn, im.im, style, window) }, - Style::None(style) => unsafe { ImeContext::create_none_ic(xconn, im, style, window) }, } .ok_or(ImeContextCreationError::Null)?; @@ -241,7 +245,7 @@ impl ImeContext { let mut context = ImeContext { ic, ic_spot: ffi::XPoint { x: 0, y: 0 }, - style, + allowed, _client_data: unsafe { Box::from_raw(client_data) }, }; @@ -348,7 +352,7 @@ impl ImeContext { } pub fn is_allowed(&self) -> bool { - !matches!(self.style, Style::None(_)) + self.allowed } // Set the spot for preedit text. Setting spot isn't working with libX11 when preedit callbacks diff --git a/src/platform_impl/linux/x11/ime/input_method.rs b/src/platform_impl/linux/x11/ime/input_method.rs index 57b47561..3a068cc2 100644 --- a/src/platform_impl/linux/x11/ime/input_method.rs +++ b/src/platform_impl/linux/x11/ime/input_method.rs @@ -82,9 +82,7 @@ impl InputMethod { } let preedit_style = preedit_style.unwrap_or_else(|| none_style.unwrap()); - // Always initialize none style even when it's not advertised, since it seems to work - // regardless... - let none_style = none_style.unwrap_or(Style::None(XIM_NONE_STYLE)); + let none_style = none_style.unwrap_or(preedit_style); Some(InputMethod { im, _name: name, preedit_style, none_style }) } diff --git a/src/platform_impl/linux/x11/ime/mod.rs b/src/platform_impl/linux/x11/ime/mod.rs index ab6702f1..0a419c8d 100644 --- a/src/platform_impl/linux/x11/ime/mod.rs +++ b/src/platform_impl/linux/x11/ime/mod.rs @@ -10,13 +10,12 @@ use std::sync::Arc; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use tracing::debug; use self::callbacks::*; use self::context::ImeContext; pub use self::context::ImeContextCreationError; use self::inner::{close_im, ImeInner}; -use self::input_method::{PotentialInputMethods, Style}; +use self::input_method::PotentialInputMethods; use super::{ffi, util, XConnection, XError}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -113,39 +112,26 @@ impl Ime { pub fn create_context( &mut self, window: ffi::Window, - with_preedit: bool, + with_ime: bool, ) -> Result { let context = if self.is_destroyed() { // 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, - im.im, - style, + im, window, None, self.inner.event_sender.clone(), + with_ime, )? }; - // Check the state on the context, since it could fail to enable or disable preedit. - let event = if matches!(style, Style::None(_)) { - if with_preedit { - debug!("failed to create IME context with preedit support.") - } - ImeEvent::Disabled - } else { - if !with_preedit { - debug!("failed to create IME context without preedit support.") - } - ImeEvent::Enabled - }; - + let event = if context.is_allowed() { ImeEvent::Enabled } else { ImeEvent::Disabled }; self.inner.event_sender.send((window, event)).expect("Failed to send enabled event"); Some(context)