Attachment 859439 Details for Bug 903702 – client/x11: Fix Key typing order

[patch] client/x11: Fix Key typing order

497f0c74230a65309e22ce5569060ce48310406b.patch (text/plain), 11.28 KB, created by poncho on 2023-04-02 16:08:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: poncho

Created: 2023-04-02 16:08:19 UTC

Size: 11.28 KB

patch

obsolete

>From 497f0c74230a65309e22ce5569060ce48310406b Mon Sep 17 00:00:00 2001
>From: fujiwarat <takao.fujiwara1@gmail.com>
>Date: Thu, 23 Mar 2023 13:07:30 +0900
>Subject: [PATCH] client/x11: Fix Key typing order
>
>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: https://github.com/ibus/ibus/commit/506ac99
>
>BUG=https://github.com/ibus/ibus/issues/2480
>---
> client/x11/main.c | 160 ++++++++++++++++++++++++----------------------
> 1 file changed, 83 insertions(+), 77 deletions(-)
>
>diff --git a/client/x11/main.c b/client/x11/main.c
>index 905fd2516..83d95cb75 100644
>--- a/client/x11/main.c
>+++ b/client/x11/main.c
>@@ -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:
>@@ -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)) { \
>@@ -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
>@@ -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 (
>@@ -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
>@@ -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);
> }
>@@ -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;
>@@ -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);
>@@ -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,
>@@ -626,7 +652,7 @@ 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 */
>@@ -634,46 +660,33 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
>                         "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,
>@@ -683,7 +696,7 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
>                                       -1,
>                                       NULL,
>                                       _process_key_event_done,
>-                                      pfe);
>+                                      data);
>         retval = 1;
>     }
>     }
>@@ -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;
>@@ -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

Actions: View | Diff

Attachments on bug 903702: 859439 | 859687