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: Tue, 18 Feb 2025 00:13:02 +0800

I'm sorry to tell you that the issue still happens with your patch. I
don't see anything abnormal about the printed timestamps when the
issue happens (no unordered timestamps found).

Now I think maybe what you described is not the same issue that I
encountered. :(

To summarize: for feature/igc at 2025/02/10 with or without the patch,
the issue happens but at a much lower probability (compared to
scratch/igc at 2024/11/24)

I think I will try bisecting between the two versions and see what affects this.

On Wed, Feb 12, 2025 at 8:36 PM Yikai Zhao <[email-masked]> wrote:
>
> 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
> >