From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#56305: 29.0.50; 'yes-or-no-p' deselects minibuffer frame Date: Sat, 9 Jul 2022 10:57:03 +0000 Message-ID: References: <83zghn7ckd.fsf@gnu.org> <83zghm5evt.fsf@gnu.org> <5d86d890-9a2e-e4d6-13fb-da03285ea003@gmx.at> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40047"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 56305@debbugs.gnu.org, Eli Zaretskii , monnier@iro.umontreal.ca, acm@muc.de To: martin rudalics Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jul 09 12:58:15 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oA8AA-000AIK-7j for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 09 Jul 2022 12:58:14 +0200 Original-Received: from localhost ([::1]:43468 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oA8A8-0001Ei-Pr for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 09 Jul 2022 06:58:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49152) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oA89y-0001DZ-UJ for bug-gnu-emacs@gnu.org; Sat, 09 Jul 2022 06:58:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:40010) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oA89y-0005af-Hy for bug-gnu-emacs@gnu.org; Sat, 09 Jul 2022 06:58:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oA89y-0001lw-EM for bug-gnu-emacs@gnu.org; Sat, 09 Jul 2022 06:58:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 09 Jul 2022 10:58:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 56305 X-GNU-PR-Package: emacs Original-Received: via spool by 56305-submit@debbugs.gnu.org id=B56305.16573642346749 (code B ref 56305); Sat, 09 Jul 2022 10:58:02 +0000 Original-Received: (at 56305) by debbugs.gnu.org; 9 Jul 2022 10:57:14 +0000 Original-Received: from localhost ([127.0.0.1]:33907 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oA89B-0001kn-ME for submit@debbugs.gnu.org; Sat, 09 Jul 2022 06:57:13 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:30523 helo=mail.muc.de) by debbugs.gnu.org with smtp (Exim 4.84_2) (envelope-from ) id 1oA899-0001kW-Cr for 56305@debbugs.gnu.org; Sat, 09 Jul 2022 06:57:11 -0400 Original-Received: (qmail 4263 invoked by uid 3782); 9 Jul 2022 10:57:04 -0000 Original-Received: from acm.muc.de (p4fe15f65.dip0.t-ipconnect.de [79.225.95.101]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Sat, 09 Jul 2022 12:57:04 +0200 Original-Received: (qmail 5914 invoked by uid 1000); 9 Jul 2022 10:57:03 -0000 Content-Disposition: inline In-Reply-To: X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:236499 Archived-At: 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).