Package: emacs;
Reported by: Daniel Colascione <dancol <at> dancol.org>
Date: Sat, 6 Aug 2022 00:55:02 UTC
Severity: normal
Message #11 received at 57012 <at> debbugs.gnu.org (full text, mbox):
From: Daniel Colascione <dancol <at> dancol.org> To: Po Lu <luangruo <at> yahoo.com> Cc: 57012 <at> debbugs.gnu.org Subject: Re: bug#57012: Activating versus raising frames Date: Sat, 06 Aug 2022 19:57:41 -0400
On Sat, 2022-08-06 at 09:44 +0800, Po Lu wrote: > Daniel Colascione <dancol <at> 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; +} + /********************************************************************* ** 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;
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.