Package: emacs;
Reported by: Yikai Zhao <yikai <at> z1k.dev>
Date: Thu, 28 Nov 2024 13:20:02 UTC
Severity: normal
Found in version 31.0.50
Message #101 received at 74590 <at> debbugs.gnu.org (full text, mbox):
From: Yikai Zhao <yikai <at> z1k.dev> To: Pip Cet <pipcet <at> protonmail.com> Cc: Po Lu <luangruo <at> yahoo.com>, Gerd Möllmann <gerd.moellmann <at> gmail.com>, Helmut Eller <eller.helmut <at> gmail.com>, 74590 <at> debbugs.gnu.org Subject: Re: bug#74590: 31.0.50 [scratch/igc branch]; key input sometimes skip fcitx input method preedit box Date: Tue, 25 Feb 2025 23:19:45 +0800
Hi,
On Sat, Feb 22, 2025 at 5:25 AM Pip Cet <pipcet <at> protonmail.com> wrote:
>
> Pip Cet <pipcet <at> protonmail.com> writes:
>
> > "Yikai Zhao" <yikai <at> z1k.dev> 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 :-)
No problem at all :) I should thank *you* all for trying to help.
I just tried this patch, it seems to fix the issue for me!
I also tried the version without the original "xev" and "CurrentTime"
hack (aka the patch below) and it still seems to work.
Just to be sure, I will keep using it for some time and let you know the result.
diff --git a/src/xterm.c b/src/xterm.c
index c3137945ac5..f34e84c52e4 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -18027,7 +18027,63 @@ 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. */
+ if ((xev->type == KeyPress || xev->type == KeyRelease)
+ && dpyinfo
+ && x_filter_event (dpyinfo, xev))
+ {
+ 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.
Regarding the test environment, there's one thing I maybe should have
mentioned earlier:
During all my testing, I'm running emacs packaged as AppImage
(https://github.com/blahgeek/emacs-appimage) because it's easier for
me to build libraries like MPS in the docker environment.
I didn't think that's relevant because AppImage is just plain emacs
executable with some extra dependencies. There's no namespaces or
other magic stuff like flatpak. Among the packaged dynamic libraries,
there's no X11 or gtk related ones either (the list is here:
https://github.com/blahgeek/emacs-appimage/blob/7d357757d90a23e42d0b5976c8ace33721915e04/scripts/postprocess.py#L60),
so there should not be some library version mismatch issue. Although,
the build environment is different from the execution environment, so
the x11 or gtk related libraries used during building are different
from the ones used during execution. But to my understanding that's
common and ok.
I'm not sure if it will help you reproduce or analyze the issue. If
so, I apologize for not mentioning it sooner.
>
> Thanks for your patience so far, I'll continue trying to trigger this on
> a virtual machine.
>
> Pip
>
Thanks
Yikai
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.