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

From: Yikai Zhao <[email-masked]>
Date: Wed, 12 Feb 2025 20:36:00 +0800

On Wed, Feb 12, 2025 at 2:21 AM Pip Cet <[email-masked]> wrote:
>
> "Yikai Zhao" <[email-masked]> writes:
>
> > Well, bad news, it still happens :(
>
> Oh no!  I spent a bit too much time on this weird bug.  First, can we
> agree that this reproduces it, even though things aren't working quite
> the way I'd expect them to?
>
> 0. install fcitx
> 1. launch Xnest :1
> 2. DISPLAY=:1 fcitx5 &
> 3. MODIFIERS=@im=fcitx DISPLAY=:1 emacs -Q
> 4. keep hitting "a" and clicking on the text cursor (not the
> black-on-black popup window)
> 5. after a while, a latin "a" will appear, once in a while, even though
> it was never selected in the popup window.
> 6. with the master branch, that doesn't happen.
>
> Does this describe the bug you're seeing?

In step 4, by "clicking on the text cursor", you mean clicking on the
text cursor in the emacs buffer by mouse, right? I'm not sure what
this action does here, my initial bug report does not include this
step. Does this increase the probability of reproducing?

The result (step 5 and 6) does match what I'm describing.

I guess what really matters is 1. fcitx popup window is present, and
2. some keypress that should go to the fcitx goes to the emacs buffer.
As long as you see these results, I think it should be the same issue?

>
> I downloaded and rebuilt Xlib to investigate this, and discovered this
> code in imDefLkup.c, which deals with "fabricated serials":
>
> Bool
> _XimIsFabricatedSerial(
>     Xim                  im,
>     XKeyEvent           *event)
> {
>     /* GTK2 XIM module sets serial=0. */
>     if (!event->serial || !im->private.proto.enable_fabricated_order)
>         return IS_FABRICATED(im) ? True : False;
>     if (event->serial == im->private.proto.fabricated_serial)
>         return True;
>     if (!im->private.proto.fabricated_serial)
>         return False;
>     /* Rotate time */
>     if (event->time < im->private.proto.fabricated_time) {
>         if (event->time >= 1000)
>             im->private.proto.fabricated_time = 0;
>     } else if (event->time - im->private.proto.fabricated_time > 1000 ) {
>         fprintf(stderr,
>                 "%s,%d: The application disposed a key event with %ld serial.\n",
>                 __FILE__, __LINE__,
>                 im->private.proto.fabricated_serial);
>         im->private.proto.enable_fabricated_order = False;
>         if (IS_FABRICATED(im)) {
>             if (event->serial)
>                 im->private.proto.fabricated_serial = event->serial;
>             return True;
>         }
>     }
>     return False;
> }
>
> As you can see, if this function is ever called for an event which
> happened *before* fabricated_time, fabricated_time is reset to 0
> (because the code assumes a 32-bit wraparound occurred), and then the
> next event triggers the warning message and, if IS_FABRICATED(im) is
> true, will be reported as fabricated, which means it will not be
> filtered, which means it will show up as an ASCII keystroke.
>
> But Emacs calls XFilterEvent in conjunction with GDK in a weird way,
> sometimes out of order (and sometimes twice for the same event? I'm not
> sure I understand this), and once in a while such reordering will result
> in the fake time wraparound happening and key presses not being
> filtered.
>
> IOW, Emacs treats XFilterEvent as a pure function, but it has side
> effects which can cause a second return value, or one generated out of
> order, to differ when input methods are in use.
>
> I assume the reason this happens with feature/igc but not with master is
> that handle_one_xevent can have significant (not user-noticeable, but
> enough for the time between keypress and keyrelease, maybe) and
> unexpected latency when GC is happening, but not with the old GC.
>
> I don't have a proper fix, but can you try this patch and see what
> behavior is like with it?

I tried this patch but I haven't reproduced the bug with this patch
yet. I will keep using it, because it also took me more time to
reproduce the issue yesterday.

In the meantime, now I feel strange that it took me a day to reproduce
the bug previously (On Feb 10, I initially reported that it seems ok,
but then I said it still happens after a longer time of usage on Feb
11). So I redid some of my testing:

- scratch/igc at 2024/11/24 (the version I used in my initial bug
report): can reproduce the issue with high probability (usually within
a few words. but less likely in the scratch buffer, more likely in a
buffer with complex content)
- feature/igc at 2025/02/10 (the version I tested days ago): cannot
reproduce within minutes of trying. (matches my report on Feb 10)

So I'm confident that the newest feature/igc version is different (at
least better) from the version at 2024/11/24. (But now I started to
worry if anything went wrong in my earlier testing... is it possible
that it's indeed fixed but I got something mixed..? I will also try to
test more and re-verify that conclusion )

Yikai

>
>
> diff --git a/src/xterm.c b/src/xterm.c
> index 80108190590..59d1d7fdaa6 100644
> --- a/src/xterm.c
> +++ b/src/xterm.c
> @@ -24131,7 +24131,8 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>                   copy.xkey.window = xev->event;
>                   copy.xkey.root = xev->root;
>                   copy.xkey.subwindow = xev->child;
> -                 copy.xkey.time = xev->time;
> +                 fprintf (stderr, "ignoring time %ld\n", (long) xev->time);
> +                 copy.xkey.time = CurrentTime;
>                   copy.xkey.state = state;
>                   xi_convert_button_state (&xev->buttons, &copy.xkey.state);
>
> @@ -24189,7 +24190,8 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>               xkey.window = xev->event;
>               xkey.root = xev->root;
>               xkey.subwindow = xev->child;
> -             xkey.time = xev->time;
> +             fprintf (stderr, "ignoring time %ld\n", (long) xev->time);
> +             xkey.time = CurrentTime;
>               xkey.state = state;
>               xkey.x = lrint (xev->event_x);
>               xkey.y = lrint (xev->event_y);
> @@ -24631,7 +24633,8 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>               xkey.window = xev->event;
>               xkey.root = xev->root;
>               xkey.subwindow = xev->child;
> -             xkey.time = xev->time;
> +             fprintf (stderr, "ignoring time %ld\n", (long) xev->time);
> +             xkey.time = CurrentTime;
>               xkey.state = xi_convert_event_keyboard_state (xev);
>               xkey.x = lrint (xev->event_x);
>               xkey.y = lrint (xev->event_y);
>
> At least over here (with the imperfect setup described above), I haven't
> been able to reproduce the falsely-unfiltered key issue.
>
> > Although I might say that it seems to happen less frequently. But my
> > memory is not reliable and there may be other factors in play...
>
> If I'm right, this is a subtle timing issue which was latent for a while
> but became an acute problem when the somewhat unpredictable GC timing of
> MPS happened.
>
> Please let me know whether the patch changes anything!
>
> Thanks!
>
> Pip
>