bug#56305: 29.0.50; 'yes-or-no-p' deselects minibuffer frame

[Alan Mackenzie <acm@muc.de>]

Hello, Martin.

On Sat, Jul 09, 2022 at 10:35:50 +0200, martin rudalics wrote:
>  >> It should not deliberately raise a frame that already has focus.

>  > OK.  We could add an extra check for the frame already having the focus.
>  > Is there anything else suboptimal about that proposed fix to emacs-28?

> If by "extra check" you mean

> diff --git a/src/minibuf.c b/src/minibuf.c
> index 0fc7f2caa1..71fd62cede 100644
> --- a/src/minibuf.c
> +++ b/src/minibuf.c
> @@ -896,6 +896,12 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
>     /* Don't allow the user to undo past this point.  */
>     bset_undo_list (current_buffer, Qnil);

> +  /* If some Emacs frame currently has the window-system focus, give
> +     it to the minibuffer frame.  This is sometimes needed for
> +     minibuffer-only frames.  */
> +  if (FRAME_DISPLAY_INFO (XFRAME (mini_frame))->x_focus_frame)
> +    Fx_focus_frame (mini_frame, Qt);
> +
>     recursive_edit_1 ();

>     /* If cursor is on the minibuffer line,

No, that's not quite what I meant.

> then it does not improve anything here - the minibuffer frame is first
> lowered and then raised above the normal frame.  I do not understand the
> idea here anyway.  Why give focus to a frame that already has focus?
> Why does the comment say "some Emacs frame" while the code checks only
> the minibuffer frame?

The intention was that FRAME_DISPLAY_INFO (XFRAME (mini_frame)) should
get the display structure which contains mini_frame, and that
->x_focus_frame should either be the Emacs frame which has the focus, or
null if some other program currently has the focus.  Only if an Emacs
frame currently has the focus should we refocus onto the minibuffer
frame.

Adding the check whether the minibuffer frame already has the focus,
which I've tried, gives this:



diff --git a/src/minibuf.c b/src/minibuf.c
index 0fc7f2caa1..0d80b2ec90 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -896,6 +896,16 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   /* Don't allow the user to undo past this point.  */
   bset_undo_list (current_buffer, Qnil);
 
+  /* If some Emacs frame currently has the window-system focus, give
+     it to the minibuffer frame.  This is sometimes needed for
+     minibuffer-only frames.  Don't give that frame the focus if it's
+     already got it, since this might cause the frame to be wrongly
+     raised.  */
+  if (FRAME_DISPLAY_INFO (XFRAME (mini_frame))->x_focus_frame
+      && (FRAME_DISPLAY_INFO (XFRAME (mini_frame))->x_focus_frame
+	  != XFRAME (mini_frame)))
+    Fx_focus_frame (mini_frame, Qt);
+
   recursive_edit_1 ();
 
   /* If cursor is on the minibuffer line,

How do you react to this suggestion?  Anyhow, I just tried it on a Linux
tty, and it segfaults.  ;-(  So it clearly needs some refinement.


> Recalling my personal experience: I used 'x-focus-frame' in one special
> case only - in 'handle-select-window' when 'focus-follows-mouse' is
> non-nil.  All other calls are via 'select-frame-set-input-focus' where
> the intention to _also_ raise the frame is obvious.

I suggested using s-f-s-input-focus at one time, but you pointed out
that this would raise the frame, which isn't wanted.

> I would never have called 'x-focus-frame' from C with the default
> settings - every second window manager out there will handle it
> differently.

But surely every window manager will give the minibuffer frame the
focus, precisely what we need here?  What could happen with a strange WM
that could be disturbing?

>  > In the mean time, how well does the change to master work?  It attempts
>  > to fix the cause of (rather than just working around) bug #56305.

> The change to master fixes the bug here.

Thanks!

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).