Subject: bug#74590: 31.0.50 [scratch/igc branch]; key input sometimes skip fcitx input method preedit box

From: Pip Cet <[email-masked]>
Date: Fri, 21 Feb 2025 21:25:33 +0000

Pip Cet <[email-masked]> writes:

> "Yikai Zhao" <[email-masked]> writes:
>
>> Hi,
>>
>> I have finished bisecting between the two versions, I have identified
>> that it's this commit that lowers the frequency of the issue:
>>
>>   commit c425f10a30c96f45fe1ccce423851bf32b8cb8df: Make balancing
>> buffer intervals optional>
>
>> Based on the change of this commit, I can also verify that, with the
>> newest version of feature/igc, when setting igc--balance-intervals to
>> t, the issue can be reproduced with similar high probability as
>> before.
>
> So it is a timing issue, most likely, we just don't know what changed
> precisely.  My next suggestion would be to add random delays near the
> buffer_step function, in order to make the bug more likely, but it's
> hard to do and I still cannot reproduce the issue you are seeing here...
>
>> I also tried your newest patch, but the issue still happens... I
>> modified your patch for a little bit to add some more debug info
>> (timestamps etc.) and recorded a video reproducing the issue. I hope
>> it helps. See attachments for the modified patch, the screencast video
>> and the log.
>
> Thank you!  That points to this commit by Po Lu in 2022:
>
> 8d0a2e4dce41fd811319e94e5f01e5fcf8b3c62a Fix build without X11 I18N
>
> * src/xterm.c (event_handler_gdk):
> (handle_one_xevent):
> (x_draw_window_cursor):
> (x_term_init): Fix build without HAVE_X_I18N.
>
> That commit adds a call to xg_filter_key to event_handler_gdk, but only
> if "current_count" is >= 0.  I have tried to read the code, but I don't
> know what "current_count" is, why it's sometimes negative, and why in
> the case that it is negative, we call handle_one_xevent without ever
> calling xg_filter_key (x_filter_key now).  That would lead to the key
> being handled, right?

I've stared at that code for a while, and I don't think it's intentional
that we're occasionally failing to call x_filter_event on key release
events.

I understand you've tried a lot of things, and you're probably running
out of patience, but if you could try this patch, in *addition* to the
one you already have, that might provide further valuable input.  Again,
I understand if you don't want to do that right now :-)

diff --git a/src/xterm.c b/src/xterm.c
index 72b681dfaaa..3a9ec542cde 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -18053,7 +18053,69 @@ event_handler_gdk (GdkXEvent *gxev, GdkEvent *ev, gpointer data)
 				current_hold_quit);
     }
   else
-    current_finish = x_dispatch_event (xev, xev->xany.display);
+    {
+      struct x_display_info *dpyinfo;
+
+      dpyinfo = x_display_info_for_display (xev->xany.display);
+
+#ifdef HAVE_X_I18N
+      /* Filter events for the current X input method.
+         GTK calls XFilterEvent but not for key press and release,
+         so we do it here.  */
+      XEvent xev_copy;
+      xev_copy = *xev;
+      DEBUG_PRINTF( "event_handler_gdk: ignoring time %ld\n",
+		    (long) xev_copy.xkey.time);
+      xev_copy.xkey.time = CurrentTime;
+      if ((xev->type == KeyPress || xev->type == KeyRelease)
+	  && dpyinfo
+         && x_filter_event (dpyinfo, &xev_copy))
+	{
+	  DEBUG_PRINTF("filtered!");
+	  unblock_input ();
+	  return GDK_FILTER_REMOVE;
+	}
+#elif USE_GTK
+      if (dpyinfo && (dpyinfo->prefer_native_input
+		      || x_gtk_use_native_input)
+	  && (xev->type == KeyPress
+#ifdef HAVE_XINPUT2
+	      /* GTK claims cookies for us, so we don't have to claim
+		 them here.  */
+	      || (dpyinfo->supports_xi2
+		  && xev->type == GenericEvent
+		  && (xev->xgeneric.extension
+		      == dpyinfo->xi2_opcode)
+		  && ((xev->xgeneric.evtype
+		       == XI_KeyPress)
+		      || (xev->xgeneric.evtype
+			  == XI_KeyRelease)))
+#endif
+	      ))
+	{
+	  struct frame *f;
+
+#ifdef HAVE_XINPUT2
+	  if (xev->type == GenericEvent)
+	    f = x_any_window_to_frame (dpyinfo,
+				       ((XIDeviceEvent *) xev->xcookie.data)->event);
+	  else
+#endif
+	    f = x_any_window_to_frame (dpyinfo, xev->xany.window);
+
+	  if (f && xg_filter_key (f, xev))
+	    {
+	      unblock_input ();
+	      return GDK_FILTER_REMOVE;
+	    }
+	}
+#endif
+
+      if (! dpyinfo)
+        current_finish = X_EVENT_NORMAL;
+      else
+	current_finish = x_dispatch_event (xev, xev->xany.display);
+    }
 
   unblock_input ();

>> In the video, I typed: c e u i <space> c e u i <space> . Each "c e u
>> i" should translates to "测试" by fcitx and "<space>" should confirm it.
>> But note that the second "c" went straight to the emacs buffer. The

The second 'c' was received by event_handler_gdk while current_count was
-1, so we never called x_filter_event for it.  I think that confuses
fcitx, which assumes all key presses and releases get filtered through
XFilterEvent.

So the patch above might fix that issue, but it doesn't explain (to me)
what current_count == -1 might mean and why it appears to happen more
often with MPS.

>> video also shows the live log on the left side, maybe it will be
>> helpful.
>
> Thanks!  The other thing I notice is that you're using GenericEvent
> events, while my setup (where I haven't been able to reproduce the bug
> after my changes) doesn't seem to produce those.  That might also be
> part of the issue.

FWIW, I've set up a qemu virtual machine to reproduce the bug, but I
haven't been successful so far.

Thanks for your patience so far, I'll continue trying to trigger this on
a virtual machine.

Pip