Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Daniel Colascione <dancol@dancol.org> writes:

> None of those things is about working against policy implemented by
> the system on behalf of the user. Your patch is explicitly about
> making Emacs defy that policy. Is that how well-behaved X clients
> should behave?

Firstly, that's not true.  Secondly, as a general rule, there are no
well-behaved X clients in existence.  Let's see what GTK does:

      /* Translate a timestamp of GDK_CURRENT_TIME appropriately */
      if (timestamp == GDK_CURRENT_TIME)
        {
#ifdef GDK_WINDOWING_X11
          if (GDK_IS_X11_WINDOW(gdk_window))
            {
              GdkDisplay *display;

              display = gtk_widget_get_display (widget);
              timestamp = gdk_x11_display_get_user_time (display);
            }
          else
#endif
            timestamp = gtk_get_current_event_time ();

[...]

      gdk_window_focus (gdk_window, timestamp);

And Qt:

    updateNetWmUserTime(connection()->time());

[...]

        event.response_type = XCB_CLIENT_MESSAGE;
        event.format = 32;
        event.sequence = 0;
        event.window = m_window;
        event.type = atom(QXcbAtom::_NET_ACTIVE_WINDOW);
        event.data.data32[0] = 1;
        event.data.data32[1] = connection()->time();
        event.data.data32[2] = focusWindow ? focusWindow->winId() : XCB_NONE;
        event.data.data32[3] = 0;
        event.data.data32[4] = 0;

(where the connection time is updated by frame drawn events, so is in
effect always very close to the server time regardless of how much
interaction has happened.)

Focus stealing prevention only works with naive clients and very old
clients, along with newly mapped windows.

> Window managers typically don't let you just raise frames
> arbitrarily. What should the documentation of this function say?

XRaiseWindow works here on GNOME Shell, kwin and mwm/dtwm, which is what
I can easily test with.  Please tell me where it doesn't work, and I
will try to fix it.

> XDestroyWindow? Windows can die at any time, yes?

They can't, not through user interaction.  Nobody defends against
extremely unruly clients that kill toplevel windows at random, even
though the fix is as simple as grabbing the server.

> Although I suppose if someone raced XDestroyWindow against
> XChangeProperty we'd either get the PropertyNotify or the xlib error
> callback would fire, depending on who won the race. I *think* we can't
> get into a situation in which we'd wait forever for the
> PropertyNotify. I'll have to double check.

A hang is possible but not something that can practically happen.  You'd
have to be a genius to time the XDestroyWindow right between the
XChangeProperty and PropertyNotify.

> It's not realistic to expect users to read any file in etc. When users
> have a problem, they try again, then ask their friends, then search
> the internet, and then give up and install VSCode. Like it or not,
> that's the flow, and checked-in documentation won't help.

Here's how VS Code (Chromium) implements activating a frame:

  x11::Time timestamp = X11EventSource::GetInstance()->GetTimestamp();

  // override_redirect windows ignore _NET_ACTIVE_WINDOW.
  // https://crbug.com/940924
  if (wm_supports_active_window && !override_redirect_) {
    std::array<uint32_t, 5> data = {
        // We're an app.
        1,
        static_cast<uint32_t>(timestamp),
        // TODO(thomasanderson): if another chrome window is active, specify
        // that here.  The EWMH spec claims this may make the WM more likely to
        // service our _NET_ACTIVE_WINDOW request.
        0,
        0,
        0,
    };
    SendClientMessage(xwindow_, x_root_window_,
                      x11::GetAtom("_NET_ACTIVE_WINDOW"), data);

(also see https://codereview.chromium.org/2287583003/ for context)

once again, the timestamp is automatically obtained from the X server
(thus bypassing any focus stealing prevention) after a few optimizations
aimed at reducing round-trips are made.

> Then let's change the documentation: add a note to x-focus-frame
> reminding callers that it's ultimately up to the window manager
> whether a window is activated --- and it is. A WM is free to ignore
> our activation requests even if we pretend to be a panel or a fire
> truck or a cool bird or whatever. Why implement three separate ways of
> working against that grain when we can do what the WM wants?

That isn't what the documentation has said for more than 20 years, so
this would in effect be an incompatible change.