bug#64152: 29.0.92; 'redirect-frame-focus' is broken

[Alan Mackenzie <acm@muc.de>]

Hello, Eli and Martin.

On Mon, Jun 19, 2023 at 20:22:23 +0300, Eli Zaretskii wrote:
> > Date: Mon, 19 Jun 2023 14:26:22 +0000
> > From: Gregory Heytings <gregory@heytings.org>
> > cc: martin rudalics <rudalics@gmx.at>, 64152@debbugs.gnu.org


> > >> That change has immediate consequences for interactions based on 
> > >> read_minibuf.  With emacs -Q evaluate (make-frame '((minibuffer . 
> > >> nil))) and in the frame created thusly type C-x C-f.  At this time it 
> > >> is impossible to access the minibuffer-less frame using C-x o, C-x 5 o, 
> > >> the window manager's Alt-TAB or any other key combination.  All these 
> > >> key combinations used to work in the given configuration ever since. 
> > >> With a minibuffer-only frames setup, the minibuffer window has become 
> > >> "modal".

> > > This works in Emacs 28, so the minibuffer-follows-selected-frame changes 
> > > aren't the culprit.  Bisection will be appreciated.


> > The culprit is 9cd72b02b6.

> Thanks.

> Alan, could you please look into fixing this?  Perhaps the recipe
> posted by Martin explains the rationale for having that "obscure,
> obsolete code" you removed?

> This is a regression in Emacs 29, so we should try fixing it on the
> emacs-29 branch.

The following patch partially reverts:

commit 9cd72b02b67e92e89b83791b66fe40c4b50d8357
Author: Alan Mackenzie <acm@muc.de>
Date:   Thu Jul 7 15:38:09 2022 +0000

    Remove obscure, obsolete code from do_switch_frame

(it doesn't restore an "#if 0 .... #else" and its "#endif").

It also reverts:

commit 0508d7c4d6637d63a823b66e9f87ab54c2e73b09
Author: Alan Mackenzie <acm@muc.de>
Date:   Fri Jul 8 20:19:03 2022 +0000

    Remove now unused parameter TRACK from do_switch_frame.

..  An extra argument (for this TRACK) had to be added into
x_try_restore_frame in xterm.c.

I'm not sure, at the moment, whether this patch unfixes any other bugs.
It runs through make check without problems.

Martin, would you please try out this patch.

Just as a matter of interest, bug #56305 (29.0.50; 'yes-or-no-p'
deselects minibuffer frame) is still open.



diff --git a/src/frame.c b/src/frame.c
index 38a6583605c..fc6a3459482 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1444,6 +1444,10 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
    If FRAME is a switch-frame event `(switch-frame FRAME1)', use
    FRAME1 as frame.
 
+   If TRACK is non-zero and the frame that currently has the focus
+   redirects its focus to the selected frame, redirect that focused
+   frame's focus to FRAME instead.
+
    FOR_DELETION non-zero means that the selected frame is being
    deleted, which includes the possibility that the frame's terminal
    is dead.
@@ -1451,7 +1455,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
    The value of NORECORD is passed as argument to Fselect_window.  */
 
 Lisp_Object
-do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
+do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object norecord)
 {
   struct frame *sf = SELECTED_FRAME (), *f;
 
@@ -1473,6 +1477,44 @@ do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
   else if (f == sf)
     return frame;
 
+  /* If the frame with GUI focus has had it's Emacs focus redirected
+     toward the currently selected frame, we should change the
+     redirection to point to the newly selected frame.  This means
+     that if the focus is redirected from a minibufferless frame to a
+     surrogate minibuffer frame, we can use `other-window' to switch
+     between all the frames using that minibuffer frame, and the focus
+     redirection will follow us around.  This code is necessary when
+     we have a minibufferless frame using the MB in another (normal)
+     frame (bug#64152) (ACM, 2023-06-20).  */
+#ifdef HAVE_WINDOW_SYSTEM
+  if (track && FRAME_WINDOW_P (f) && FRAME_TERMINAL (f)->get_focus_frame)
+    {
+      Lisp_Object gfocus; /* The frame which still has focus on the
+			     current terminal, according to the GUI
+			     system. */
+      Lisp_Object focus;  /* The frame to which Emacs has redirected
+			     the focus from `gfocus'.  This might be a
+			     frame with a minibuffer when `gfocus'
+			     doesn't have a MB.  */
+
+      gfocus = FRAME_TERMINAL (f)->get_focus_frame (f);
+      if (FRAMEP (gfocus))
+	{
+	  focus = FRAME_FOCUS_FRAME (XFRAME (gfocus));
+	  if (FRAMEP (focus) && XFRAME (focus) == SELECTED_FRAME ())
+	      /* Redirect frame focus also when FRAME has its minibuffer
+		 window on the selected frame (see Bug#24500).
+
+		 Don't do that: It causes redirection problem with a
+		 separate minibuffer frame (Bug#24803) and problems
+		 when updating the cursor on such frames.
+	      || (NILP (focus)
+		  && EQ (FRAME_MINIBUF_WINDOW (f), sf->selected_window)))  */
+	    Fredirect_frame_focus (gfocus, frame);
+	}
+    }
+#endif /* HAVE_X_WINDOWS */
+
   if (!for_deletion && FRAME_HAS_MINIBUF_P (sf))
     resize_mini_window (XWINDOW (FRAME_MINIBUF_WINDOW (sf)), 1);
 
@@ -1574,7 +1616,7 @@ DEFUN ("select-frame", Fselect_frame, Sselect_frame, 1, 2, "e",
     /* Do not select a tooltip frame (Bug#47207).  */
     error ("Cannot select a tooltip frame");
   else
-    return do_switch_frame (frame, 0, norecord);
+    return do_switch_frame (frame, 1, 0, norecord);
 }
 
 DEFUN ("handle-switch-frame", Fhandle_switch_frame,
@@ -1590,7 +1632,7 @@ DEFUN ("handle-switch-frame", Fhandle_switch_frame,
   kset_prefix_arg (current_kboard, Vcurrent_prefix_arg);
   run_hook (Qmouse_leave_buffer_hook);
 
-  return do_switch_frame (event, 0, Qnil);
+  return do_switch_frame (event, 0, 0, Qnil);
 }
 
 DEFUN ("selected-frame", Fselected_frame, Sselected_frame, 0, 0, 0,
@@ -2108,7 +2150,7 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 	Fraise_frame (frame1);
 #endif
 
-      do_switch_frame (frame1, 1, Qnil);
+      do_switch_frame (frame1, 0, 1, Qnil);
       sf = SELECTED_FRAME ();
     }
   else
diff --git a/src/keyboard.c b/src/keyboard.c
index b1ccf4acde4..99f886821e2 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -11561,7 +11561,7 @@ quit_throw_to_read_char (bool from_signal)
   if (FRAMEP (internal_last_event_frame)
       && !EQ (internal_last_event_frame, selected_frame))
     do_switch_frame (make_lispy_switch_frame (internal_last_event_frame),
-		     0, Qnil);
+		     0, 0, Qnil);
 
   sys_longjmp (getcjmp, 1);
 }
diff --git a/src/lisp.h b/src/lisp.h
index 9c02d975a74..bf91a1559bf 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4878,7 +4878,7 @@ fast_c_string_match_ignore_case (Lisp_Object regexp,
 /* Defined in frame.c.  */
 extern void store_frame_param (struct frame *, Lisp_Object, Lisp_Object);
 extern void store_in_alist (Lisp_Object *, Lisp_Object, Lisp_Object);
-extern Lisp_Object do_switch_frame (Lisp_Object, int, Lisp_Object);
+extern Lisp_Object do_switch_frame (Lisp_Object, int, int, Lisp_Object);
 extern Lisp_Object get_frame_param (struct frame *, Lisp_Object);
 extern void frames_discard_buffer (Lisp_Object);
 extern void init_frame_once (void);
diff --git a/src/minibuf.c b/src/minibuf.c
index bcb7eb9375d..6e54d8c3ba5 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1125,8 +1125,8 @@ read_minibuf_unwind (void)
  found:
   if (!EQ (exp_MB_frame, saved_selected_frame)
       && !NILP (exp_MB_frame))
-    do_switch_frame (exp_MB_frame, 0, Qt); /* This also sets
-					      minibuf_window */
+    do_switch_frame (exp_MB_frame, 0, 0, Qt); /* This also sets
+					     minibuf_window */
 
   /* To keep things predictable, in case it matters, let's be in the
      minibuffer when we reset the relevant variables.  Don't depend on
@@ -1238,7 +1238,7 @@ read_minibuf_unwind (void)
   /* Restore the selected frame. */
   if (!EQ (exp_MB_frame, saved_selected_frame)
       && !NILP (exp_MB_frame))
-    do_switch_frame (saved_selected_frame, 0, Qt);
+    do_switch_frame (saved_selected_frame, 0, 0, Qt);
 }
 
 /* Replace the expired minibuffer in frame exp_MB_frame with the next less
diff --git a/src/window.c b/src/window.c
index 0efd6813f8d..1dc977626b3 100644
--- a/src/window.c
+++ b/src/window.c
@@ -7399,7 +7399,7 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
 	do_switch_frame (NILP (dont_set_frame)
                          ? data->selected_frame
                          : old_frame
-                         , 0, Qnil);
+                         , 0, 0, Qnil);
     }
 
   FRAME_WINDOW_CHANGE (f) = true;
diff --git a/src/xterm.c b/src/xterm.c
index e981a36fa9c..5840b15bcb7 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -25792,7 +25792,7 @@ x_try_restore_frame (void)
 
   FOR_EACH_FRAME (tail, frame)
     {
-      if (!NILP (do_switch_frame (frame, 1, Qnil)))
+      if (!NILP (do_switch_frame (frame, 0, 1, Qnil)))
 	return;
     }
 }


-- 
Alan Mackenzie (Nuremberg, Germany).