bug#57012: Activating versus raising frames

bug#57012: Activating versus raising frames

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

I noticed that raising an Emacs frame (latest master) with emacsclient does 
nothing under current Cinnamon. Emacs gains focus, but the focused window 
isn't raised.

I can reproduce the problem with xdotool's windowraise command, which 
similarly did nothing. xdotool windowactivate works though, so I'm guessing 
the EWMH activate code in xterm.c would similarly do the trick. Emacs used 
to use EWMH activate on frame raise but stopped in 2007 to work around 
deadlocks in a version of metacity in use at the time. Can we once again 
activate Emacs frames on raise?

[-- Attachment #2: Type: text/html, Size: 771 bytes --]

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> I noticed that raising an Emacs frame (latest master) with emacsclient
> does nothing under current Cinnamon. Emacs gains focus, but the
> focused window isn't raised.
>
> I can reproduce the problem with xdotool's windowraise command, which
> similarly did nothing. xdotool windowactivate works though, so I'm
> guessing the EWMH activate code in xterm.c would similarly do the
> trick. Emacs used to use EWMH activate on frame raise but stopped in
> 2007 to work around deadlocks in a version of metacity in use at the
> time. Can we once again activate Emacs frames on raise?

Wouldn't it make more sense for emacsclient to focus the frame, which
does activate it by default?

bug#57012: Activating versus raising frames

[Daniel Colascione]

On Sat, 2022-08-06 at 09:44 +0800, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > I noticed that raising an Emacs frame (latest master) with
> > emacsclient
> > does nothing under current Cinnamon. Emacs gains focus, but the
> > focused window isn't raised.
> > 
> > I can reproduce the problem with xdotool's windowraise command,
> > which
> > similarly did nothing. xdotool windowactivate works though, so
> > I'm
> > guessing the EWMH activate code in xterm.c would similarly do the
> > trick. Emacs used to use EWMH activate on frame raise but stopped
> > in
> > 2007 to work around deadlocks in a version of metacity in use at
> > the
> > time. Can we once again activate Emacs frames on raise?
> 
> Wouldn't it make more sense for emacsclient to focus the frame,
> which
> does activate it by default?
> 

Hoo boy. I spent a bit of time digging into the event code. The root
cause of the inability of emacsclient to raise the frame is that
we've been getting X11 event timestamps wrong for some time. In
particular, 1) in GTK builds, we're not updating the X timestamps for
keyboard and mouse input events, and 2) we're not updating the X
timestamp when we get an emacsclient request.  Because of #2, when
we call x-focus-window in select-frame-set-input-focus, the timestamp
we send along with the _NET_ACTIVE_WINDOW request is stale, causing
some window managers (e.g. cinnamon and kwin) to just ignore the
_NET_ACTIVE_WINDOW. But because we use _NET_ACTIVE_WINDOW *and*
XSetInputFocus and the latter works, the overall effect is that the
call to select-frame-set-input-focus in server.el focuses the Emacs
window, but doesn't raise it.

The following patch should fix both problems:

diff --git a/lisp/server.el b/lisp/server.el
index a06f2f952f..cd3a8f80f0 100644
--- 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)))))
 
 (defvar server-stop-automatically nil
   "Internal status variable for `server-stop-automatically'.")
diff --git a/src/frame.c b/src/frame.c
index 25d71e0769..084df8ef21 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -5942,6 +5942,25 @@ DEFUN ("frame--set-was-invisible",
Fframe__set_was_invisible,
 
   return f->was_invisible ? Qt : Qnil;
 }
+
+DEFUN ("frame-note-oob-interaction",
+       Fframe_note_oob_interaction,
+       Sframe_note_oob_interaction, 0, 1, 0,
+       doc: /* Note that the user has interacted with a frame.
+This function is useful when the user interacts with Emacs out-of-
band
+(e.g., via the server) and we want to pretend for purposes of Emacs
+interacting with the window system that the last interaction time
was
+the time of that out-of-band interaction, not the time of the last
+window system input event delivered to that frame.  */)
+  (Lisp_Object frame)
+{
+  struct frame *f = decode_any_frame (frame);
+  if (FRAME_LIVE_P (f) &&
+      FRAME_TERMINAL (f)->note_oob_interaction_hook)
+    FRAME_TERMINAL (f)->note_oob_interaction_hook (f);
+  return Qnil;
+}
+
 \f
/*********************************************************************
**
 			Multimonitor data
@@ -6626,6 +6645,7 @@ focus (where a frame immediately loses focus
when it's left by the mouse
   defsubr (&Sframe_window_state_change);
   defsubr (&Sset_frame_window_state_change);
   defsubr (&Sframe_scale_factor);
+  defsubr (&Sframe_note_oob_interaction);
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sx_get_resource);
diff --git a/src/gtkutil.c b/src/gtkutil.c
index a6bba096a4..b2af5ff5c2 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -6658,6 +6658,17 @@ xg_filter_key (struct frame *frame, XEvent
*xkey)
 }
 #endif
 
+#ifndef HAVE_PGTK
+void
+xg_set_user_timestamp (struct frame *frame, guint32 time)
+{
+  GtkWidget *widget = FRAME_GTK_OUTER_WIDGET (frame);
+  GdkWindow *window = gtk_widget_get_window (widget);
+  eassert (window);
+  gdk_x11_window_set_user_time (window, time);
+}
+#endif
+
 #if GTK_CHECK_VERSION (3, 10, 0)
 static void
 xg_widget_style_updated (GtkWidget *widget, gpointer user_data)
diff --git a/src/gtkutil.h b/src/gtkutil.h
index 190d662831..bca7ea8176 100644
--- a/src/gtkutil.h
+++ b/src/gtkutil.h
@@ -224,6 +224,10 @@ #define XG_ITEM_DATA "emacs_menuitem"
 extern bool xg_filter_key (struct frame *frame, XEvent *xkey);
 #endif
 
+#ifndef HAVE_PGTK
+extern void xg_set_user_timestamp (struct frame *frame, guint32
time);
+#endif
+
 /* Mark all callback data that are Lisp_Objects during GC.  */
 extern void xg_mark_data (void);
 
diff --git a/src/termhooks.h b/src/termhooks.h
index c5f1e286e9..2c204afeca 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -860,6 +860,13 @@ #define EVENT_INIT(event) (memset (&(event), 0,
sizeof (struct input_event)), \
      will be considered as grabbed.  */
   bool (*any_grab_hook) (Display_Info *);
 #endif
+
+  /* Called to note that the user has interacted with a window
system
+     frame outside the window system and that we should update the
+     window system's notion of the user's last interaction time with
+     that frame.  */
+  void (*note_oob_interaction_hook) (struct frame *);
+
 } GCALIGNED_STRUCT;
 
 INLINE bool
diff --git a/src/xterm.c b/src/xterm.c
index 4bbcfb0e59..4d42f30e20 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -6581,12 +6581,6 @@ x_set_frame_alpha (struct frame *f)
   x_stop_ignoring_errors (dpyinfo);
 }
 
-
/*********************************************************************
**
-		    Starting and ending an update
-
*********************************************************************
**/
-
-#if defined HAVE_XSYNC && !defined USE_GTK
-
 /* Wait for an event matching PREDICATE to show up in the event
    queue, or TIMEOUT to elapse.
 
@@ -6640,6 +6634,12 @@ x_if_event (Display *dpy, XEvent
*event_return,
     }
 }
 
+/*******************************************************************
****
+		    Starting and ending an update
+
*********************************************************************
**/
+
+#if defined HAVE_XSYNC && !defined USE_GTK
+
 /* Return the monotonic time corresponding to the high-resolution
    server timestamp TIMESTAMP.  Return 0 if the necessary
information
    is not available.  */
@@ -7521,26 +7521,25 @@ x_draw_fringe_bitmap (struct window *w,
struct glyph_row *row,
    user time.  We don't sanitize timestamps from events sent by the
X
    server itself because some Lisp might have set the user time to a
    ridiculously large value, and this way a more reasonable
timestamp
-   can be obtained upon the next event.  */
+   can be obtained upon the next event.  If EXPLICIT_FRAME is NULL,
+   update the focused frame's timestamp; otherwise, update
+   EXPLICIT_FRAME's. */
 
 static void
-x_display_set_last_user_time (struct x_display_info *dpyinfo, Time
time,
-			      bool send_event)
+x_display_set_last_user_time_1 (struct x_display_info *dpyinfo, Time
time,
+				bool send_event,
+				struct frame *explicit_frame)
 {
-#ifndef USE_GTK
-  struct frame *focus_frame;
+  struct frame *frame;
   Time old_time;
-#if defined HAVE_XSYNC
+#if defined HAVE_XSYNC && !defined USE_GTK
   uint64_t monotonic_time;
 #endif
 
-  focus_frame = dpyinfo->x_focus_frame;
+  frame = explicit_frame ? explicit_frame : dpyinfo->x_focus_frame;
   old_time = dpyinfo->last_user_time;
-#endif
 
-#ifdef ENABLE_CHECKING
   eassert (time <= X_ULONG_MAX);
-#endif
 
   if (!send_event || time > dpyinfo->last_user_time)
     dpyinfo->last_user_time = time;
@@ -7567,23 +7566,35 @@ x_display_set_last_user_time (struct
x_display_info *dpyinfo, Time time,
     }
 #endif
 
-#ifndef USE_GTK
-  /* Don't waste bandwidth if the time hasn't actually changed.  */
-  if (focus_frame && old_time != dpyinfo->last_user_time)
+  /* Don't waste bandwidth if the time hasn't actually changed.
+     Update anyway if we're updating the timestamp for a non-focused
+     frame, since the event loop might not have gotten around to
+     updating that frame's timestamp.  */
+  if (frame && (explicit_frame || old_time != dpyinfo-
>last_user_time))
     {
       time = dpyinfo->last_user_time;
 
-      while (FRAME_PARENT_FRAME (focus_frame))
-	focus_frame = FRAME_PARENT_FRAME (focus_frame);
+      while (FRAME_PARENT_FRAME (frame))
+	frame = FRAME_PARENT_FRAME (frame);
 
-      if (FRAME_X_OUTPUT (focus_frame)->user_time_window != None)
+#if defined USE_GTK
+      xg_set_user_timestamp (frame, time);
+#else
+      if (FRAME_X_OUTPUT (frame)->user_time_window != None)
 	XChangeProperty (dpyinfo->display,
-			 FRAME_X_OUTPUT (focus_frame)-
>user_time_window,
+			 FRAME_X_OUTPUT (frame)->user_time_window,
 			 dpyinfo->Xatom_net_wm_user_time,
 			 XA_CARDINAL, 32, PropModeReplace,
 			 (unsigned char *) &time, 1);
-    }
 #endif
+    }
+}
+
+static void
+x_display_set_last_user_time (struct x_display_info *dpyinfo, Time
time,
+			      bool send_event)
+{
+  x_display_set_last_user_time_1 (dpyinfo, time, send_event, NULL);
 }
 
 /* Not needed on GTK because GTK handles reporting the user time
@@ -25859,9 +25870,11 @@ xembed_request_focus (struct frame *f)
 			 XEMBED_REQUEST_FOCUS, 0, 0, 0);
 }
 
-/* Activate frame with Extended Window Manager Hints */
+/* Activate frame with Extended Window Manager Hints
 
-static void
+Return whether we were successful in doing so.  */
+
+static bool
 x_ewmh_activate_frame (struct frame *f)
 {
   XEvent msg;
@@ -25869,8 +25882,7 @@ x_ewmh_activate_frame (struct frame *f)
 
   dpyinfo = FRAME_DISPLAY_INFO (f);
 
-  if (FRAME_VISIBLE_P (f)
-      && x_wm_supports (f, dpyinfo->Xatom_net_active_window))
+  if (x_wm_supports (f, dpyinfo->Xatom_net_active_window))
     {
       /* See the documentation at
 	
https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html
@@ -25890,7 +25902,9 @@ x_ewmh_activate_frame (struct frame *f)
       XSendEvent (dpyinfo->display, dpyinfo->root_window,
 		  False, (SubstructureRedirectMask
 			  | SubstructureNotifyMask), &msg);
+      return true;
     }
+  return false;
 }
 
 static Lisp_Object
@@ -25928,16 +25942,14 @@ x_focus_frame (struct frame *f, bool
noactivate)
        events.  See XEmbed Protocol Specification at
        https://freedesktop.org/wiki/Specifications/xembed-spec/  */
     xembed_request_focus (f);
-  else
+  else if (noactivate ||
+	   (!FRAME_PARENT_FRAME (f) && !x_ewmh_activate_frame (f)))
     {
       /* 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)
-	x_ewmh_activate_frame (f);
     }
 }
 
@@ -28552,6 +28564,54 @@ x_have_any_grab (struct x_display_info
*dpyinfo)
 }
 #endif
 
+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;
+}
+
+static bool
+x_get_server_time (struct frame* f, Time* time)
+{
+  struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (f);
+  Atom property_atom = dpyinfo->Xatom_EMACS_SERVER_TIME_PROP;
+  XEvent event;
+
+  XChangeProperty (dpyinfo->display, FRAME_OUTER_WINDOW (f),
+		   property_atom, XA_ATOM, 32,
+		   PropModeReplace, (unsigned char *)
&property_atom, 1);
+
+  if (x_if_event (dpyinfo->display, &event,
server_timestamp_predicate,
+		  (XPointer) &(XID[]) {FRAME_OUTER_WINDOW (f),
property_atom},
+		  dtotimespec (XFLOAT_DATA
(Vx_wait_for_event_timeout))))
+    return false;
+  *time = event.xproperty.time;
+  return true;
+}
+
+static void
+x_note_oob_interaction (struct frame *f)
+{
+  while (FRAME_PARENT_FRAME (f))
+    f = FRAME_PARENT_FRAME (f);
+  if (FRAME_LIVE_P (f)) {
+    Time server_time;
+    if (!x_get_server_time (f, &server_time))
+      error ("Timed out waiting for server timestamp");
+    x_display_set_last_user_time_1 (
+      FRAME_DISPLAY_INFO (f), server_time, false, f);
+  }
+}
+
 /* Create a struct terminal, initialize it with the X11 specific
    functions and make DISPLAY->TERMINAL point to it.  */
 
@@ -28622,6 +28682,7 @@ x_create_terminal (struct x_display_info
*dpyinfo)
 #ifdef HAVE_XINPUT2
   terminal->any_grab_hook = x_have_any_grab;
 #endif
+  terminal->note_oob_interaction_hook = x_note_oob_interaction;
   /* Other hooks are NULL by default.  */
 
   return terminal;

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> Hoo boy. I spent a bit of time digging into the event code. The root
> cause of the inability of emacsclient to raise the frame is that
> we've been getting X11 event timestamps wrong for some time. In
> particular, 1) in GTK builds, we're not updating the X timestamps for
> keyboard and mouse input events

The reason for that is because GTK is supposed to do that itself, after
the event ends up dispatched to GDK.  I will investigate this further.

> , and 2) we're not updating the X timestamp when we get an emacsclient
> request.  Because of #2, when we call x-focus-window in
> select-frame-set-input-focus, the timestamp we send along with the
> _NET_ACTIVE_WINDOW request is stale, causing some window managers
> (e.g. cinnamon and kwin) to just ignore the _NET_ACTIVE_WINDOW. But
> because we use _NET_ACTIVE_WINDOW *and* XSetInputFocus and the latter
> works, the overall effect is that the call to
> select-frame-set-input-focus in server.el focuses the Emacs window,
> but doesn't raise it.

That sounds likely to me, thanks for investigating.

> The following patch should fix both problems:

Right, but I saw a similar problem in the DND code, so I'd prefer you
modified `x-display-set-last-user-time' instead.  I think adding a
separate FRAME argument to that function would be in order.  Aside from
that, this is too X-specific to warrant a terminal hook.

The rest of the code is fine by me (tho there is a
dpyinfo->server_time_monotonic_p flag that can be used to avoid a sync
with the X server when trying to obtain the server time), but needs
coding style fixes.  I'm sure you already know how to do that.

bug#57012: Activating versus raising frames

[Daniel Colascione]

On 8/6/22 21:55, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
>
>> Hoo boy. I spent a bit of time digging into the event code. The root
>> cause of the inability of emacsclient to raise the frame is that
>> we've been getting X11 event timestamps wrong for some time. In
>> particular, 1) in GTK builds, we're not updating the X timestamps for
>> keyboard and mouse input events
> The reason for that is because GTK is supposed to do that itself, after
> the event ends up dispatched to GDK.  I will investigate this further.

The GDK code specifically mentions that programs that handle events 
themselves (like Emacs) need to explicitly update the event time (as my 
patch does)

>> , and 2) we're not updating the X timestamp when we get an emacsclient
>> request.  Because of #2, when we call x-focus-window in
>> select-frame-set-input-focus, the timestamp we send along with the
>> _NET_ACTIVE_WINDOW request is stale, causing some window managers
>> (e.g. cinnamon and kwin) to just ignore the _NET_ACTIVE_WINDOW. But
>> because we use _NET_ACTIVE_WINDOW *and* XSetInputFocus and the latter
>> works, the overall effect is that the call to
>> select-frame-set-input-focus in server.el focuses the Emacs window,
>> but doesn't raise it.
> That sounds likely to me, thanks for investigating.
>
>> The following patch should fix both problems:
> Right, but I saw a similar problem in the DND code,

What is the bug?

> so I'd prefer you
> modified `x-display-set-last-user-time' instead.  I think adding a
> separate FRAME argument to that function would be in order.

We could do that, sure.

>   Aside from
> that, this is too X-specific to warrant a terminal hook.

Sorry, but I strongly disagree. The concept of signaling to the 
underlying window system that the user has interacted in some manner 
with a frame is generic and not X-specific. In fact --- doesn't the pgtk 
backend need an implementation of this hook too? It, like the 
conventional GTK backend, is blind to interactions with the frame 
performed using emacsclient.

> The rest of the code is fine by me (tho there is a
> dpyinfo->server_time_monotonic_p flag that can be used to avoid a sync
> with the X server when trying to obtain the server time),

I think server_time_monotonic_p is an unnecessary optimization.

 > but needs coding style fixes. I'm sure you already know how to do that.

The style is fine, thanks.

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> The GDK code specifically mentions that programs that handle events
> themselves (like Emacs) need to explicitly update the event time (as
> my patch does)

The GDK documentation is unclear.  You only have to update the event
time if the event is not passed to GDK, by setting *finish to
X_EVENT_DROP, which really only happens with key press events.

> What is the bug?

Client messages sent to x-dnd.el did not automatically update the user
time, causing various selection-related functions to use an outdated
timestamp.

> Sorry, but I strongly disagree. The concept of signaling to the
> underlying window system that the user has interacted in some manner
> with a frame is generic and not X-specific. In fact --- doesn't the
> pgtk backend need an implementation of this hook too? It, like the
> conventional GTK backend, is blind to interactions with the frame
> performed using emacsclient.

No, the PGTK backend doesn't have a concept of "server time".  The GDK
Wayland backend implements them via event serials, which cannot be
generated.  It is also unnecessary to specify the server time when
trying to activate a toplevel window.

The only window system I know of that requires that to be specified is
X, so let's keep the code specific to X.

> I think server_time_monotonic_p is an unnecessary optimization.

It's used during frame synchronization, which naturally requires highly
accurate views of the server time.  Further more, we try to _reduce_ the
amount of calls to XSync, which leads to slow performance over display
connections with high latency.

> The style is fine, thanks.

No, it's not:

> +static bool
> +x_get_server_time (struct frame* f, Time* time)

should be

> +x_get_server_time (struct frame* f, Time *time)

and

> +  if (FRAME_LIVE_P (f)) {
> +    Time server_time;
> +    if (!x_get_server_time (f, &server_time))
> +      error ("Timed out waiting for server timestamp");
> +    x_display_set_last_user_time_1 (
> +      FRAME_DISPLAY_INFO (f), server_time, false, f);
> +  }

Should have the opening braces on a new line.

Thanks.

bug#57012: Activating versus raising frames

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]



On August 6, 2022 22:45:26 Po Lu <luangruo@yahoo.com> wrote:

> Daniel Colascione <dancol@dancol.org> writes:
>
>> The GDK code specifically mentions that programs that handle events
>> themselves (like Emacs) need to explicitly update the event time (as
>> my patch does)
>
> The GDK documentation is unclear.  You only have to update the event
> time if the event is not passed to GDK, by setting *finish to
> X_EVENT_DROP, which really only happens with key press events.
>
>> What is the bug?
>
> Client messages sent to x-dnd.el did not automatically update the user
> time, causing various selection-related functions to use an outdated
> timestamp.
>
>> Sorry, but I strongly disagree. The concept of signaling to the
>> underlying window system that the user has interacted in some manner
>> with a frame is generic and not X-specific. In fact --- doesn't the
>> pgtk backend need an implementation of this hook too? It, like the
>> conventional GTK backend, is blind to interactions with the frame
>> performed using emacsclient.
>
> No, the PGTK backend doesn't have a concept of "server time".  The GDK
> Wayland backend implements them via event serials, which cannot be
> generated.  It is also unnecessary to specify the server time when
> trying to activate a toplevel window.
>
> The only window system I know of that requires that to be specified is
> X, so let's keep the code specific to X.

pgtk also runs on X, and the problem must be solved there in some manner. 
GTK has no magic facility for knowing that emacsclient ran. Regardless, a 
terminal hook is not expensive, and I don't want to add yet more window 
system typecases to the code. Terminal access should be polymorphic. It's 
through terminal hooks that we make them polymorphic. I'm not removing the 
terminal hook.

[-- Attachment #2: Type: text/html, Size: 3409 bytes --]

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> pgtk also runs on X, and the problem must be solved there in some
> manner.

It does not.  We do not support running the PGTK build on X (the
selection code doesn't work on X, for example), and there is no way to
"touch" the user time on that platform without relying on X11-specific
code.  At present, it's not even possible to include gdk/gdkx.h there
due to typedef conflicts with dispextern.h.

> GTK has no magic facility for knowing that emacsclient
> ran. Regardless, a terminal hook is not expensive, and I don't want to
> add yet more window system typecases to the code. Terminal access
> should be polymorphic. It's through terminal hooks that we make them
> polymorphic. I'm not removing the terminal hook.

After thinking a bit, I figure that a better way to solve the problem
would be to document that window managers don't always respect
x-focus-frame, and to add a force parameter which makes it query for the
current server time and set it as the user time, thus making focus
setting more reliable.

Thanks.

bug#57012: Activating versus raising frames

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]



On August 6, 2022 23:03:04 Po Lu <luangruo@yahoo.com> wrote:

> Daniel Colascione <dancol@dancol.org> writes:
>
>> pgtk also runs on X, and the problem must be solved there in some
>> manner.
>
> It does not.  We do not support running the PGTK build on X (the
> selection code doesn't work on X, for example), and there is no way to
> "touch" the user time on that platform without relying on X11-specific
> code.  At present, it's not even possible to include gdk/gdkx.h there
> due to typedef conflicts with dispextern.h

I'm surprised to hear that considering that many other GTK applications 
manage selections adequately. If the intent of pgtk is to run only on 
Wayland, you should break the pgtk build at runtime if it's running under 
X11, and probably rename it too --- because "pure GTK" sounds like it 
should rely only on things GTK provides and that it should therefore run 
anywhere GTK does. If in fact it's just a Wayland window system 
implementation, call it that.

>
>
>> GTK has no magic facility for knowing that emacsclient
>> ran. Regardless, a terminal hook is not expensive, and I don't want to
>> add yet more window system typecases to the code. Terminal access
>> should be polymorphic. It's through terminal hooks that we make them
>> polymorphic. I'm not removing the terminal hook.
>
> After thinking a bit, I figure that a better way to solve the problem
> would be to document that window managers don't always respect
> x-focus-frame, and to add a force parameter which makes it query for the
> current server time and set it as the user time, thus making focus
> setting more reliable.


I don't agree. Telling Emacs that a user has interacted with a frame is not 
an X specific concept. And even in the context of X11, we should be 
resetting the user time generally, not just hacking something up for the 
special case of x-focus-frame, because 1) the general approach preserves 
timestamp monotonicity, and 2) the user did in fact interact with the frame.


>
> Thanks.


[-- Attachment #2: Type: text/html, Size: 3954 bytes --]

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> On August 6, 2022 23:03:04 Po Lu <luangruo@yahoo.com> wrote:
>
>  Daniel Colascione <dancol@dancol.org> writes:
>
>  pgtk also runs on X, and the problem must be solved there in some
>  manner.
>
>  It does not.  We do not support running the PGTK build on X (the
>  selection code doesn't work on X, for example), and there is no way to
>  "touch" the user time on that platform without relying on X11-specific
>  code.  At present, it's not even possible to include gdk/gdkx.h there
>  due to typedef conflicts with dispextern.h
>
> I'm surprised to hear that considering that many other GTK
> applications manage selections adequately. If the intent of pgtk is to
> run only on Wayland, you should break the pgtk build at runtime if
> it's running under X11, and probably rename it too --- because "pure
> GTK" sounds like it should rely only on things GTK provides and that
> it should therefore run anywhere GTK does. If in fact it's just a
> Wayland window system implementation, call it that.

It does break at runtime when run under X11: just type "C-x h" in a
large file (like xdisp.c), and try to insert the region into another
program with mouse-2.

Other GTK programs run well because simply don't provide the amount of
features that Emacs does.  Their users don't notice various problems
caused by GTK, including "C-S-u" being read as "C-u", or "kp-home" being
translated by the input method into "home".  But our users do, which is
why we do not support X11 on the PGTK builds, since the regular X build
works much better.

It's documented to only support window systems that aren't X11, such as
Wayland and Broadway.

> I don't agree. Telling Emacs that a user has interacted with a frame
> is not an X specific concept.

The point I'm making is that telling Emacs that a user has "interacted"
with a frame should not be necessary at all, and should not be part of
an API exposed anywhere.  Lisp code calls `x-focus-frame'; as a result,
the frame is focused and activated.

Further more, modern window managers are rather notorious for their
draconian "focus stealing prevention", which is probably what we are
running into here.  Kwin and Mutter apparently ignore all
_NET_ACTIVE_WINDOW requests that don't come from a pager when the window
being activated belongs to a program different from that of the
currently active window.

> And even in the context of X11, we should be resetting the user time
> generally, not just hacking something up for the special case of
> x-focus-frame, because 1) the general approach preserves timestamp
> monotonicity, and 2) the user did in fact interact with the frame.

Some data being written to an Emacs server socket does not or a timer
calling `focus-frame' does not qualify as "user interaction" and is not
sufficient reason to set the user time.

Timestamp monotonicity does not really matter either, since we are not
dealing with X selections, drag-and-drop or other fancy synchronization
mechanisms.

bug#57012: Activating versus raising frames

[Daniel Colascione]

On Sun, 2022-08-07 at 11:29 +0800, Po Lu wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > On August 6, 2022 23:03:04 Po Lu <luangruo@yahoo.com> wrote:
> > 
> >  Daniel Colascione <dancol@dancol.org> writes:
> > 
> >  pgtk also runs on X, and the problem must be solved there in some
> >  manner.
> > 
> >  It does not.  We do not support running the PGTK build on X (the
> >  selection code doesn't work on X, for example), and there is no way to
> >  "touch" the user time on that platform without relying on X11-specific
> >  code.  At present, it's not even possible to include gdk/gdkx.h there
> >  due to typedef conflicts with dispextern.h
> > 
> > I'm surprised to hear that considering that many other GTK
> > applications manage selections adequately. If the intent of pgtk is to
> > run only on Wayland, you should break the pgtk build at runtime if
> > it's running under X11, and probably rename it too --- because "pure
> > GTK" sounds like it should rely only on things GTK provides and that
> > it should therefore run anywhere GTK does. If in fact it's just a
> > Wayland window system implementation, call it that.
> 
> It does break at runtime when run under X11: just type "C-x h" in a
> large file (like xdisp.c), and try to insert the region into another
> program with mouse-2.
> 
> Other GTK programs run well because simply don't provide the amount of
> features that Emacs does.  Their users don't notice various problems
> caused by GTK, including "C-S-u" being read as "C-u", or "kp-home" being
> translated by the input method into "home".  But our users do, which is
> why we do not support X11 on the PGTK builds, since the regular X build
> works much better.
> 
> It's documented to only support window systems that aren't X11, such as
> Wayland and Broadway.

Then emit an error message at runtime if you detect them running on
X11 telling users that things will mysteriously. To let them build
and run pgtk initially, then get frustrated with a few edge cases, is
doing them a disservice. They're not going to read that
documentation.

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> Then emit an error message at runtime if you detect them running on
> X11 telling users that things will mysteriously. To let them build
> and run pgtk initially, then get frustrated with a few edge cases, is
> doing them a disservice. They're not going to read that
> documentation.

Why won't they?

bug#57012: Activating versus raising frames

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]



On August 7, 2022 00:29:36 Po Lu <luangruo@yahoo.com> wrote:

> Daniel Colascione <dancol@dancol.org> writes:
>
>> Then emit an error message at runtime if you detect them running on
>> X11 telling users that things will mysteriously. To let them build
>> and run pgtk initially, then get frustrated with a few edge cases, is
>> doing them a disservice. They're not going to read that
>> documentation.
>
> Why won't they?


Because, as a pragmatic matter, they don't. We're lucky if users read fatal 
error messages. People don't have time to trawl through documentation 
before trying something. They read about some cool new compilation mode on 
Reddit (scanning just the post titles of course), them try it. It's not 
that they're dumb. They've busy. It doesn't matter whether some file 
somewhere says a scenario isn't supported: users are going to try that 
scenario anyway if it plausibly looks like it might work, and when it 
doesn't, they're going to think less of the whole project. I'd feel much 
better if pgtk simply refused to start under X (explaining that it's not 
supported) than silently give users a poor experience.


[-- Attachment #2: Type: text/html, Size: 2075 bytes --]

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Colascione <dancol@dancol.org> writes:

> Because, as a pragmatic matter, they don't. We're lucky if users read
> fatal error messages. People don't have time to trawl through
> documentation before trying something. They read about some cool new
> compilation mode on Reddit (scanning just the post titles of course),
> them try it. It's not that they're dumb. They've busy. It doesn't
> matter whether some file somewhere says a scenario isn't supported:
> users are going to try that scenario anyway if it plausibly looks like
> it might work, and when it doesn't, they're going to think less of the
> whole project. I'd feel much better if pgtk simply refused to start
> under X (explaining that it's not supported) than silently give users
> a poor experience.

How about adding a warning to configure.ac instead?  Do you think those
users will read that?

bug#57012: Activating versus raising frames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

I installed the change I proposed earlier, since Emacs 29 should not be
released with this broken, and it's sat on the list since August without
serious technical objections.