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

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

[martin rudalics]

The doc-string of 'redirect-frame-focus' says:

   A frame’s focus redirection can be changed by ‘select-frame’.  If frame
   FOO is selected, and then a different frame BAR is selected, any
   frames redirecting their focus to FOO are shifted to redirect their
   focus to BAR.  This allows focus redirection to work properly when the
   user switches from one frame to another using ‘select-window’.

   This means that a frame whose focus is redirected to itself is treated
   differently from a frame whose focus is redirected to nil; the former
   is affected by ‘select-frame’, while the latter is not.

The Elisp manual says almost the same:

      Selecting a frame can also change focus redirections.  Selecting
      frame ‘bar’, when ‘foo’ had been selected, changes any redirections
      pointing to ‘foo’ so that they point to ‘bar’ instead.  This allows
      focus redirection to work properly when the user switches from one
      frame to another using ‘select-window’.

      This means that a frame whose focus is redirected to itself is
      treated differently from a frame whose focus is not redirected.
      ‘select-frame’ affects the former but not the latter.

With Emacs 29 both texts have become false.  Selecting a frame no longer
shifts focus to that frame.

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".

martin


In GNU Emacs 29.0.92 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5,
  cairo version 1.16.0) of 2023-06-18 built on restno
Repository revision: a24e9e3fee59435422af0473b7ec585de2c13b4e
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

Configured using:
  'configure --with-gif=ifavailable --with-tiff=ifavailable
  --with-gnutls=no --without-pop 'CFLAGS=-O3 -no-pie''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GSETTINGS HARFBUZZ JPEG LIBSELINUX MODULES
NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND THREADS TOOLKIT_SCROLL_BARS X11
XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: de_AT.utf8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 36164 6573)
  (symbols 48 5127 0)
  (strings 32 13109 2112)
  (string-bytes 1 370199)
  (vectors 16 9293)
  (vector-slots 8 148348 10817)
  (floats 8 22 23)
  (intervals 56 210 0)
  (buffers 976 10))

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

[Eli Zaretskii]

> Date: Sun, 18 Jun 2023 16:26:56 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
> With Emacs 29 both texts have become false.  Selecting a frame no longer
> shifts focus to that frame.
> 
> 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.

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

[Gregory Heytings]

>> 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.

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

[Eli Zaretskii]

> 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.

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

[Alan Mackenzie]

Hello, Eli.

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?

Will do.

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

OK.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Alan Mackenzie]

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).

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

[martin rudalics]

 > Martin, would you please try out this patch.

It fixes the bug.  Thanks.

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

We probably should provide an option to turn off frame redirection in
read_minibuf in the first place.  People with 'minibuffer-auto-raise'
non-nil need that option like a hole in the head.

martin

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

[Alan Mackenzie]

Hello, Eli.

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.

It's fixed on the emacs-29 branch (Martin has tested the patch).  Should
I commit it to the release branch?

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Eli Zaretskii]

> Date: Wed, 21 Jun 2023 12:22:18 +0000
> Cc: Gregory Heytings <gregory@heytings.org>, rudalics@gmx.at,
>   64152@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > This is a regression in Emacs 29, so we should try fixing it on the
> > emacs-29 branch.
> 
> It's fixed on the emacs-29 branch (Martin has tested the patch).  Should
> I commit it to the release branch?

Yes, please.

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

[Alan Mackenzie]

Hello, Eli and Martin.

On Wed, Jun 21, 2023 at 15:50:51 +0300, Eli Zaretskii wrote:
> > Date: Wed, 21 Jun 2023 12:22:18 +0000
> > Cc: Gregory Heytings <gregory@heytings.org>, rudalics@gmx.at,
> >   64152@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>

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

> > It's fixed on the emacs-29 branch (Martin has tested the patch).  Should
> > I commit it to the release branch?

> Yes, please.

DONE.

I'm closing the bug with this post.

-- 
Alan Mackenzie (Nuremberg, Germany).