Re: master 4b98a79a50: Improve X event timestamp tracking

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm as far from understanding the fine technical details of X
> interaction as it gets, but from the POV of an interested bystander,
> it sounds like the practical difference between you two is whether in
> the following snippet:
>
>   --- a/lisp/server.el
>   +++ b/lisp/server.el
>   @@ -1721,7 +1721,9 @@ server-switch-buffer
>               ;; a minibuffer/dedicated-window (if there's no
>   other).
>               (error (pop-to-buffer next-buffer)))))))
>        (when server-raise-frame
>   -      (select-frame-set-input-focus (window-frame)))))
>   +      (let ((frame (window-frame)))
>   +        (frame-note-oob-interaction frame)
>   +        (select-frame-set-input-focus frame)))))
>
> we should call a special function that Daniel suggests to add, or
> introduce a new optional argument to select-frame-set-input-focus to
> force raising the frame, is that right?

Right, but after digging some more I would rather just make
x-focus-frame activate the frame by default, as long as noactivate is
non-nil.  As documented.

> You also say that the issue is more general than just that of raising
> a frame when it gets focus, but do we actually know about other
> situations where that could be an issue?  If we do, then indeed a more
> general approach is more beneficial, IMO.

I can't think of any more general issue here.

Anyway, this patch makes x-focus-frame work as documented.  Daniel,
please see if it works for you (on and outside GTK) with the original
recipe in the bug report:

diff --git a/src/xfns.c b/src/xfns.c
index 672097c0d8..267e2a824c 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -2578,6 +2578,7 @@ append_wm_protocols (struct x_display_info *dpyinfo,
 #if !defined HAVE_GTK3 && defined HAVE_XSYNC
   bool found_wm_sync_request = false;
 #endif
+  bool found_wm_take_focus = false;
   unsigned long bytes_after;
 
   block_input ();
@@ -2601,6 +2602,9 @@ append_wm_protocols (struct x_display_info *dpyinfo,
                   == dpyinfo->Xatom_net_wm_sync_request)
            found_wm_sync_request = true;
 #endif
+         else if (existing_protocols[nitems]
+                  == dpyinfo->Xatom_wm_take_focus)
+           found_wm_take_focus = true;
        }
     }
 
@@ -2609,6 +2613,8 @@ append_wm_protocols (struct x_display_info *dpyinfo,
 
   if (!found_wm_ping)
     protos[num_protos++] = dpyinfo->Xatom_net_wm_ping;
+  if (!found_wm_take_focus)
+    protos[num_protos++] = dpyinfo->Xatom_wm_take_focus;
 #if !defined HAVE_GTK3 && defined HAVE_XSYNC
   if (!found_wm_sync_request && dpyinfo->xsync_supported_p)
     protos[num_protos++] = dpyinfo->Xatom_net_wm_sync_request;
diff --git a/src/xterm.c b/src/xterm.c
index 97985c8d9e..b5487f7e9d 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -17295,6 +17295,10 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                   }
                 /* Not certain about handling scroll bars here */
 #endif
+               /* Set the last focus time to the time provided inside
+                  the _WM_TAKE_FOCUS message.  */
+
+               dpyinfo->last_focus_time = event->xclient.data.l[1];
                goto done;
               }
 
@@ -25883,6 +25887,44 @@ xembed_request_focus (struct frame *f)
                         XEMBED_REQUEST_FOCUS, 0, 0, 0);
 }
 
+static Bool
+server_timestamp_predicate (Display *display,
+                           XEvent *xevent,
+                           XPointer arg)
+{
+  XID *args = (XID *) arg;
+
+  if (xevent->type == PropertyNotify
+      && xevent->xproperty.window == args[0]
+      && xevent->xproperty.atom == args[1])
+    return True;
+
+  return False;
+}
+
+/* Get the server time.  The X server is guaranteed to deliver the
+   PropertyNotify event, so there is no reason to use x_if_event.  */
+
+static Time
+x_get_server_time (struct frame *f)
+{
+  Atom property_atom;
+  XEvent property_dummy;
+  struct x_display_info *dpyinfo;
+
+  dpyinfo = FRAME_DISPLAY_INFO (f);
+  property_atom = dpyinfo->Xatom_EMACS_SERVER_TIME_PROP;
+
+  XChangeProperty (dpyinfo->display, FRAME_OUTER_WINDOW (f),
+                  property_atom, XA_ATOM, 32,
+                  PropModeReplace, (unsigned char *) &property_atom, 1);
+
+  XIfEvent (dpyinfo->display, &property_dummy, server_timestamp_predicate,
+           (XPointer) &(XID[]) {FRAME_OUTER_WINDOW (f), property_atom});
+
+  return property_dummy.xproperty.time;
+}
+
 /* Activate frame with Extended Window Manager Hints */
 
 static void
@@ -25890,11 +25932,11 @@ x_ewmh_activate_frame (struct frame *f)
 {
   XEvent msg;
   struct x_display_info *dpyinfo;
+  Time time;
 
   dpyinfo = FRAME_DISPLAY_INFO (f);
 
-  if (FRAME_VISIBLE_P (f)
-      && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
+  if (FRAME_VISIBLE_P (f))
     {
       /* See the documentation at
         https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html
@@ -25904,13 +25946,52 @@ x_ewmh_activate_frame (struct frame *f)
       msg.xclient.message_type = dpyinfo->Xatom_net_active_window;
       msg.xclient.format = 32;
       msg.xclient.data.l[0] = 1;
-      msg.xclient.data.l[1] = dpyinfo->last_user_time;
+      msg.xclient.data.l[1] = (dpyinfo->last_user_time > 
dpyinfo->last_focus_time
+                              ? dpyinfo->last_user_time
+                              : dpyinfo->last_focus_time);
       msg.xclient.data.l[2] = (!dpyinfo->x_focus_frame
                               ? None
                               : FRAME_OUTER_WINDOW (dpyinfo->x_focus_frame));
       msg.xclient.data.l[3] = 0;
       msg.xclient.data.l[4] = 0;
 
+      /* No frame is currently focused on that display, so apply any
+        bypass for focus stealing prevention that the user has
+        specified.  */
+      if (!dpyinfo->x_focus_frame)
+       {
+         if (EQ (Vx_allow_focus_stealing, Qimitate_pager))
+           msg.xclient.data.l[0] = 2;
+         else if (EQ (Vx_allow_focus_stealing, Qnewer_time))
+           {
+             block_input ();
+             time = x_get_server_time (f);
+#ifdef USE_GTK
+             x_set_gtk_user_time (f, time);
+#endif
+             /* Temporarily override dpyinfo->x_focus_frame so the
+                user time property is set on the right window.  */
+             dpyinfo->x_focus_frame = f;
+             x_display_set_last_user_time (dpyinfo, time, true);
+             dpyinfo->x_focus_frame = NULL;
+             unblock_input ();
+
+             msg.xclient.data.l[1] = time;
+           }
+         else if (EQ (Vx_allow_focus_stealing, Qraise_and_focus))
+           {
+             time = x_get_server_time (f);
+
+             x_ignore_errors_for_next_request (dpyinfo);
+             XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
+                             RevertToParent, time);
+             XRaiseWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f));
+             x_stop_ignoring_errors (dpyinfo);
+
+             return;
+           }
+       }
+
       XSendEvent (dpyinfo->display, dpyinfo->root_window,
                  False, (SubstructureRedirectMask
                          | SubstructureNotifyMask), &msg);
@@ -25954,14 +26035,19 @@ x_focus_frame (struct frame *f, bool noactivate)
     xembed_request_focus (f);
   else
     {
-      /* Ignore any BadMatch error this request might result in.  */
-      x_ignore_errors_for_next_request (dpyinfo);
-      XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
-                     RevertToParent, CurrentTime);
-      x_stop_ignoring_errors (dpyinfo);
-
-      if (!noactivate)
+      if (!noactivate && NILP (Vx_no_window_manager)
+         && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
+       /* Calling XSetInputFocus manually can be skipped, since
+          activating a frame means to make it the input focus.  */
        x_ewmh_activate_frame (f);
+      else
+       {
+         /* Ignore any BadMatch error this request might result in.  */
+         x_ignore_errors_for_next_request (dpyinfo);
+         XSetInputFocus (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
+                         RevertToParent, CurrentTime);
+         x_stop_ignoring_errors (dpyinfo);
+       }
     }
 }
 
@@ -29068,6 +29154,9 @@ syms_of_xterm (void)
   Fput (Qsuper, Qmodifier_value, make_fixnum (super_modifier));
   DEFSYM (QXdndSelection, "XdndSelection");
   DEFSYM (Qx_selection_alias_alist, "x-selection-alias-alist");
+  DEFSYM (Qimitate_pager, "imitate-pager");
+  DEFSYM (Qnewer_time, "newer-time");
+  DEFSYM (Qraise_and_focus, "raise-and-focus");
 
   DEFVAR_LISP ("x-ctrl-keysym", Vx_ctrl_keysym,
     doc: /* Which keys Emacs uses for the ctrl modifier.
@@ -29294,4 +29383,23 @@ syms_of_xterm (void)
 In addition, when this variable is a list, only preserve the
 selections whose names are contained within.  */);
   Vx_auto_preserve_selections = list2 (QCLIPBOARD, QPRIMARY);
+
+  DEFVAR_LISP ("x-allow-focus-stealing", Vx_allow_focus_stealing,
+    doc: /* How to bypass window manager focus stealing prevention.
+
+Some window managers prevent `x-focus-frame' from activating the given
+frame when Emacs is in the background.  This variable specifies the
+strategy used to activate frames when that is the case, and has
+several valid values (any other value means to not bypass window
+manager focus stealing prevention):
+
+  - The symbol `imitate-pager', which means to pretend that Emacs is a
+    pager.
+
+  - The symbol `newer-time', which means to fetch the current time
+    from the X server and use it to activate the frame.
+
+  - The symbol `raise-and-focus', which means to raise the window and
+    focus it manually.  */);
+  Vx_allow_focus_stealing = Qnewer_time;
 }
diff --git a/src/xterm.h b/src/xterm.h
index 7be0f2ede6..b7f8edb98f 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -538,8 +538,9 @@ #define MAX_CLIP_RECTS 2
   struct scroll_bar *last_mouse_scroll_bar;
 
   /* Time of last user interaction as returned in X events on this
-     display.  */
-  Time last_user_time;
+     display, and the time of the last window-manager assisted
+     focus.  */
+  Time last_user_time, last_focus_time;
 
   /* Position where the mouse was last time we reported a motion.
      This is a position on last_mouse_motion_frame.  */