Skip to content

Commit

Permalink
client/x11: Fix Key typing order
Browse files Browse the repository at this point in the history 
ibus-x11 now also uses the hybrid process key events with
IBUS_ENABLE_SYNC_MODE=2 and it waits for the async API
with GSource and g_main_context_iteration() in xim_forward_event().

But g_main_context_iteration() calls gdk_event_source_dispatch()
and it can call another xim_forward_event() and the callbacks
of ibus_input_context_process_key_event_async() can be nested.
So if the forwarding API is called out of the callbacks of
ibus_input_context_process_key_event_async(), the key events
order is swapped due to the delayed return of
g_main_context_iteration().

To resolve this issue, the forwarding API should be called in
the callbacks of ibus_input_context_process_key_event_async().

Fixes: 506ac99

BUG=#2480
  • Loading branch information
@fujiwarat
fujiwarat committed Mar 23, 2023
1 parent ce5e2bb commit 497f0c7
Showing 1 changed file with 83 additions and 77 deletions.
160 changes: 83 additions & 77 deletions client/x11/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/* vim:set et sts=4: */
/* ibus
* Copyright (C) 2007-2015 Peng Huang <shawn.p.huang@gmail.com>
* Copyright (C) 2015-2022 Takao Fujiwara <takao.fujiwara1@gmail.com>
* Copyright (C) 2015-2023 Takao Fujiwara <takao.fujiwara1@gmail.com>
* Copyright (C) 2007-2015 Red Hat, Inc.
*
* main.c:
Expand Down Expand Up @@ -49,6 +49,8 @@
#include <getopt.h>

#define ESC_SEQUENCE_ISO10646_1 "\033%G"
/* Wait for about 120 secs to return a key from async process-key-event. */
#define MAX_WAIT_KEY_TIME 120000

#define LOG(level, fmt_args...) \
if (g_debug_level >= (level)) { \
Expand Down Expand Up @@ -461,11 +463,39 @@ xim_unset_ic_focus (XIMS xims, IMChangeFocusStruct *call_data)

}

static void
_xim_forward_key_event_done (X11IC *x11ic,
XEvent *event,
gboolean processed)
{
IMForwardEventStruct fe;
if (processed) {
if (!x11ic->has_preedit_area) {
_xim_set_cursor_location (x11ic);
}
return;
}
g_assert (x11ic);
g_assert (event);

memset (&fe, 0, sizeof (fe));
fe.major_code = XIM_FORWARD_EVENT;
fe.icid = x11ic->icid;
fe.connect_id = x11ic->connect_id;
fe.sync_bit = 0;
fe.serial_number = 0L;
fe.event = *event;
IMForwardEvent (_xims, (XPointer) &fe);
}


typedef struct {
IMForwardEventStruct *pfe;
int count;
guint count_cb_id;
gboolean retval;
X11IC *x11ic;
CARD16 connect_id;
XEvent event;
} ProcessKeyEventReplyData;

static void
Expand All @@ -474,7 +504,7 @@ _process_key_event_done (GObject *object,
gpointer user_data)
{
IBusInputContext *context = (IBusInputContext *)object;
IMForwardEventStruct *pfe = (IMForwardEventStruct*) user_data;
ProcessKeyEventReplyData *data = (ProcessKeyEventReplyData *)user_data;

GError *error = NULL;
gboolean retval = ibus_input_context_process_key_event_async_finish (
Expand All @@ -488,16 +518,15 @@ _process_key_event_done (GObject *object,
}

if (g_hash_table_lookup (_connections,
GINT_TO_POINTER ((gint) pfe->connect_id))
GINT_TO_POINTER ((gint)data->connect_id))
== NULL) {
g_slice_free (IMForwardEventStruct, pfe);
g_slice_free (ProcessKeyEventReplyData, data);
return;
}

if (retval == FALSE) {
IMForwardEvent (_xims, (XPointer) pfe);
}
g_slice_free (IMForwardEventStruct, pfe);
if (retval == FALSE)
_xim_forward_key_event_done (data->x11ic, &data->event, retval);
g_slice_free (ProcessKeyEventReplyData, data);
}

static void
Expand All @@ -518,6 +547,21 @@ _process_key_event_reply_done (GObject *object,
}
g_return_if_fail (data);
data->retval = retval;
if (g_hash_table_lookup (_connections,
GINT_TO_POINTER ((gint)data->connect_id))
== NULL) {
return;
}
/* _xim_forward_key_event_done() should be called in
* _process_key_event_reply_done() because g_main_context_iteration()
* can call another xim_forward_event() and xim_forward_event() can be
* nested and the first _process_key_event_reply_done() is returned
* at last with g_main_context_iteration() so
* if _xim_forward_key_event_done() is called out of
* _process_key_event_reply_done(), the key events order
* can be swapped.
*/
_xim_forward_key_event_done (data->x11ic, &data->event, retval);
data->count = 0;
g_source_remove (data->count_cb_id);
}
Expand All @@ -529,9 +573,8 @@ _process_key_event_count_cb (gpointer user_data)
g_return_val_if_fail (data, G_SOURCE_REMOVE);
if (!data->count)
return G_SOURCE_REMOVE;
/* Wait for about 10 secs. */
if (data->count++ == 10000) {
data->count = 0;
if (data->count++ == MAX_WAIT_KEY_TIME) {
g_warning ("Key event is not returned for %usecs.", MAX_WAIT_KEY_TIME);
return G_SOURCE_REMOVE;
}
return G_SOURCE_CONTINUE;
Expand Down Expand Up @@ -571,32 +614,13 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
event.keyval,
event.hardware_keycode - 8,
event.state);
if (retval) {
if (!x11ic->has_preedit_area) {
_xim_set_cursor_location (x11ic);
}
return 1;
}

IMForwardEventStruct fe;
memset (&fe, 0, sizeof (fe));

fe.major_code = XIM_FORWARD_EVENT;
fe.icid = x11ic->icid;
fe.connect_id = x11ic->connect_id;
fe.sync_bit = 0;
fe.serial_number = 0L;
fe.event = call_data->event;

IMForwardEvent (_xims, (XPointer) &fe);

_xim_forward_key_event_done (x11ic, &call_data->event, retval);
retval = 1;
break;
}
case 2: {
GSource *source = g_timeout_source_new (1);
ProcessKeyEventReplyData *data = NULL;
IMForwardEventStruct fe;

if (source)
data = g_slice_new0 (ProcessKeyEventReplyData);
Expand All @@ -610,11 +634,13 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
if (source)
g_source_destroy (source);
} else {
CARD16 connect_id = x11ic->connect_id;
data->count = 1;
g_source_attach (source, NULL);
g_source_unref (source);
data->count_cb_id = g_source_get_id (source);
data->connect_id = call_data->connect_id;
data->x11ic = x11ic;
data->event = *((XEvent*)xevent);
ibus_input_context_process_key_event_async (
x11ic->context,
event.keyval,
Expand All @@ -626,54 +652,41 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
data);
g_source_set_callback (source, _process_key_event_count_cb,
data, NULL);
while (data->count)
while (data->count > 0 && data->count < MAX_WAIT_KEY_TIME)
g_main_context_iteration (NULL, TRUE);
if (source->ref_count > 0) {
/* g_source_get_id() could causes a SEGV */
g_info ("Broken GSource.ref_count and maybe a timing "
"issue in %p.", source);
}
retval = data->retval;
g_slice_free (ProcessKeyEventReplyData, data);

if (g_hash_table_lookup (_connections,
GINT_TO_POINTER ((gint)connect_id))
== NULL) {
if (data->count == 0) {
g_slice_free (ProcessKeyEventReplyData, data);
return 1;
}
}

if (retval) {
if (! x11ic->has_preedit_area) {
_xim_set_cursor_location (x11ic);
}
return 1;
g_slice_free (ProcessKeyEventReplyData, data);
if (g_hash_table_lookup (_connections,
GINT_TO_POINTER ((gint)call_data->connect_id))
== NULL) {
return 1;
}

memset (&fe, 0, sizeof (fe));

fe.major_code = XIM_FORWARD_EVENT;
fe.icid = x11ic->icid;
fe.connect_id = x11ic->connect_id;
fe.sync_bit = 0;
fe.serial_number = 0L;
fe.event = call_data->event;

IMForwardEvent (_xims, (XPointer) &fe);

_xim_forward_key_event_done (x11ic, &call_data->event, retval);
retval = 1;
break;
}
default: {
IMForwardEventStruct *pfe;
ProcessKeyEventReplyData *data;

pfe = g_slice_new0 (IMForwardEventStruct);
pfe->major_code = XIM_FORWARD_EVENT;
pfe->icid = x11ic->icid;
pfe->connect_id = x11ic->connect_id;
pfe->sync_bit = 0;
pfe->serial_number = 0L;
pfe->event = call_data->event;
if (!(data = g_slice_new0 (ProcessKeyEventReplyData))) {
g_warning ("Cannot allocate async data");
_xim_forward_key_event_done (x11ic, &call_data->event, 0);
return 1;
}
data->connect_id = call_data->connect_id;
data->x11ic = x11ic;
data->event = call_data->event;

ibus_input_context_process_key_event_async (
x11ic->context,
Expand All @@ -683,7 +696,7 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
-1,
NULL,
_process_key_event_done,
pfe);
data);
retval = 1;
}
}
Expand Down Expand Up @@ -962,11 +975,10 @@ _xim_forward_key_event (X11IC *x11ic,
guint keycode,
guint state)
{
g_return_if_fail (x11ic != NULL);

IMForwardEventStruct fe = {0};
XEvent xkp = {0};

g_return_if_fail (x11ic != NULL);

xkp.xkey.type = (state & IBUS_RELEASE_MASK) ? KeyRelease : KeyPress;
xkp.xkey.serial = 0L;
xkp.xkey.send_event = False;
Expand All @@ -975,20 +987,14 @@ _xim_forward_key_event (X11IC *x11ic,
xkp.xkey.window =
x11ic->focus_window ? x11ic->focus_window : x11ic->client_window;
xkp.xkey.subwindow = None;
xkp.xkey.root = DefaultRootWindow (GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()));
xkp.xkey.root = DefaultRootWindow (
GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()));

xkp.xkey.time = 0;
xkp.xkey.state = state;
xkp.xkey.keycode = (keycode == 0) ? 0 : keycode + 8;

fe.major_code = XIM_FORWARD_EVENT;
fe.icid = x11ic->icid;
fe.connect_id = x11ic->connect_id;
fe.sync_bit = 0;
fe.serial_number = 0L;
fe.event = xkp;

IMForwardEvent (_xims, (XPointer) & fe);
_xim_forward_key_event_done (x11ic, &xkp, FALSE);
}

static void
Expand Down

0 comments on commit 497f0c7

Please sign in to comment.