bug#48674: Frames and minibuffer bug

bug#48674: Frames and minibuffer bug

[Iris García]

[-- Attachment #1: Type: text/plain, Size: 4684 bytes --]

Hi everyone, this is the first time I report a bug.
In order to reproduce the bug I did exactly the following:
1. Start emacs: emacs -Q
2. Paste the following code in the scratch buffer:
(defvar box-cursor t)

(defun test/set-cursor()
  "Set cursor in all frames depending on the active state."
  (interactive)
  (dolist (frame (frame-list))
    (with-selected-frame frame
      (if box-cursor
          (progn
            (modify-frame-parameters
             frame (list (cons 'cursor-type 'box)))
            (modify-frame-parameters
             frame (list (cons 'cursor-color "#00A9FE"))))
        (progn
          (modify-frame-parameters
           frame (list (cons 'cursor-type 'hbar)))
          (modify-frame-parameters
           frame (list (cons 'cursor-color "green")))
          )))))

(defun test/enter-minibuffer()
  (setq box-cursor nil)
  (test/set-cursor))

(defun test/exit-minibuffer()
  (setq box-cursor t)
  (test/set-cursor))


(add-hook 'minibuffer-setup-hook #'test/enter-minibuffer)
(add-hook 'minibuffer-exit-hook #'test/exit-minibuffer)

(server-start)
(make-frame)

;; Call M-x (execute-extended-command) and you will see:
;; It is not possible to write in the minibuffer
;; Killing the previously created frame seems to solve the issue.

3. M-x -> eval-buffer -> RET
4. M-x (The bug appears, it is not possible to write in the minibuffer)

Note:
As soon as the new frame is destroyed, it is possible again to write in the
minibuffer.

Info:
In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.29,
cairo version 1.17.4)
 of 2021-05-26 built on ryzen9
Repository revision: f4dc646e0d7fb673f3149836bb7299fba9539e80
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Arch Linux

Configured using:
 'configure --with-native-compilation --with-mailutils --with-pop
 --with-x --with-json --with-xwidgets --with-imagemagick --with-cairo'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
IMAGEMAGICK JPEG JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LC_CTYPE: C
  value of $LC_MESSAGES: C
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source eieio eieio-core eieio-loaddefs
password-cache json map text-property-search time-date mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail comp comp-cstr warnings subr-x rx cl-seq cl-macs cl-extra
help-mode seq byte-opt gv cl-loaddefs cl-lib bytecomp byte-compile cconv
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils server
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type 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 elisp-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 cl-generic
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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
x multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 88884 6685)
 (symbols 48 7886 1)
 (strings 32 22435 3140)
 (string-bytes 1 860986)
 (vectors 16 15992)
 (vector-slots 8 273880 13488)
 (floats 8 30 175)
 (intervals 56 499 0)
 (buffers 992 13))

[-- Attachment #2: Type: text/html, Size: 5422 bytes --]

bug#48674: Frames and minibuffer bug

[martin rudalics]

merge 48674 48675
quit

Thanks for the report.  I've just tried to merge this with the following
one.

 > 4. M-x (The bug appears, it is not possible to write in the minibuffer)

The crucial part is the

   (dolist (frame (frame-list))
     (with-selected-frame frame

setting the cursor is not necessary to reproduce it.  Since the bug does
not appear with Emacs 27, this could be due to Alan's minibuffer window
changes.

 > Note:
 > As soon as the new frame is destroyed, it is possible again to write in the
 > minibuffer.

Right but as soon as I make another frame and type M-x I get

completing-read-default: Command attempted to use minibuffer while in minibuffer

so I'm at the same time in the active minibuffer and a normal buffer.

martin

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Iris and Martin.

Iris, thanks for taking the trouble to report this bug, and thanks even
more for cutting the test case down to the absolute minimum, making the
bug easier to diagnose.

On Wed, May 26, 2021 at 19:45:11 +0200, martin rudalics wrote:
> merge 48674 48675
> quit

> Thanks for the report.  I've just tried to merge this with the following
> one.

>  > 4. M-x (The bug appears, it is not possible to write in the minibuffer)

> The crucial part is the

>    (dolist (frame (frame-list))
>      (with-selected-frame frame

> setting the cursor is not necessary to reproduce it.  Since the bug does
> not appear with Emacs 27, this could be due to Alan's minibuffer window
> changes.

Sort of, almost.

What is happening is that the with-selected-frame invocation is
selecting (temporarily) a different frame from the minibuffer's frame.
This has the (intended) side effect of making the MB no longer selected
in that frame.  When the MB's frame becomes selected again, nothing
makes the mini-window the selected window.  This needs fixing.

I think it likely that this bug could be triggered in Emacs 27 by
putting something into minibuffer-setup-hook that selects a different
window in the MB's frame, though I haven't tried this.

>  > Note:
>  > As soon as the new frame is destroyed, it is possible again to write in the
>  > minibuffer.

> Right but as soon as I make another frame and type M-x I get

> completing-read-default: Command attempted to use minibuffer while in minibuffer

> so I'm at the same time in the active minibuffer and a normal buffer.

Yes.  I think the following patch should fix the trouble.  Iris, could
you please apply the patch (in directory ..../emacs/src) and rebuild
your Emacs-28, then try it out on your real lisp source code.  Should
you want any help with the patching or building, feel free to send me
private email.  Then please confirm that the problem is fixed, or tell
us what's still not working.  Thanks!

Martin, that Qt in the Fselect_window call (the NORECORD argument) -
would it be perhaps be better as Qnil?


diff --git a/src/minibuf.c b/src/minibuf.c
index cffb7fe787..3468643a7e 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -893,6 +893,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   run_hook (Qminibuffer_setup_hook);
 
+  /* If the above hook has made the mini-window no longer the selected
+     window, restore it.  */
+  if (!EQ (selected_window, minibuf_window))
+    Fselect_window (minibuf_window , Qt);
+
   /* Don't allow the user to undo past this point.  */
   bset_undo_list (current_buffer, Qnil);
 


> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[martin rudalics]

 > What is happening is that the with-selected-frame invocation is
 > selecting (temporarily) a different frame from the minibuffer's frame.
 > This has the (intended) side effect of making the MB no longer selected
 > in that frame.  When the MB's frame becomes selected again, nothing
 > makes the mini-window the selected window.  This needs fixing.

Does this mean that the

   Fselect_window (f->selected_window, norecord);

in do_switch_frame fails?  If so, why?  Do we anywhere violate the

   (eq (selected-window) (frame-selected-window (selected-frame)))

invariant?  That might be fatal.  Both, `with-selected-frame' and
`with-selected-window', should leave no traces behind.

 > Martin, that Qt in the Fselect_window call (the NORECORD argument) -
 > would it be perhaps be better as Qnil?
 >
 >
 > diff --git a/src/minibuf.c b/src/minibuf.c
 > index cffb7fe787..3468643a7e 100644
 > --- a/src/minibuf.c
 > +++ b/src/minibuf.c
 > @@ -893,6 +893,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 >
 >     run_hook (Qminibuffer_setup_hook);
 >
 > +  /* If the above hook has made the mini-window no longer the selected
 > +     window, restore it.  */
 > +  if (!EQ (selected_window, minibuf_window))
 > +    Fselect_window (minibuf_window , Qt);
 > +

Are we sure that we want to disallow a function on
`minibuffer-setup-hook' to change the selected window?  With Emacs 27


(defun foo ()
   (select-window (frame-first-window)))

(add-hook 'minibuffer-setup-hook 'foo)


works without any problems here.

The NORECORD argument is important only if you need it - so far, the
previous buffers of the minibuffer window were largely ignored.

martin

bug#48674: Frames and minibuffer bug

[Iris García]

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

Hi Martin,

I forgot to include you in my last mail where I said:

I think I have found the new issue (it is related to the former one), my
> code this time was the following:
>
> (defvar box-cursor t)
>
> (defun test/set-cursor()
>   "Set cursor in all frames depending on the active state."
>   (interactive)
>   (dolist (frame (frame-list))
>     (with-selected-frame frame
>       (if box-cursor
>           (progn
>             (modify-frame-parameters
>              frame (list (cons 'cursor-type 'box)))
>             (modify-frame-parameters
>              frame (list (cons 'cursor-color "#00A9FE"))))
>         (progn
>           (modify-frame-parameters
>            frame (list (cons 'cursor-type 'hbar)))
>           (modify-frame-parameters
>            frame (list (cons 'cursor-color "green")))
>           )))))
>
> (defun test/enter-minibuffer()
>   (setq box-cursor nil)
>   (test/set-cursor))
>
> (defun test/exit-minibuffer()
>   (setq box-cursor t)
>   (test/set-cursor))
>
>
> (add-hook 'window-state-change-hook #'test/enter-minibuffer)
> (add-hook 'window-state-change-hook #'test/exit-minibuffer)
>
> (server-start)
> (make-frame
>
> The only difference is the add-hook, this time using
> window-state-change-hook instead of minibuffer-...
> This leads to the same bug.
>
> Regards,
>
> Iris.
>

On Thu, 27 May 2021 at 16:33, martin rudalics <rudalics@gmx.at> wrote:

>  > What is happening is that the with-selected-frame invocation is
>  > selecting (temporarily) a different frame from the minibuffer's frame.
>  > This has the (intended) side effect of making the MB no longer selected
>  > in that frame.  When the MB's frame becomes selected again, nothing
>  > makes the mini-window the selected window.  This needs fixing.
>
> Does this mean that the
>
>    Fselect_window (f->selected_window, norecord);
>
> in do_switch_frame fails?  If so, why?  Do we anywhere violate the
>
>    (eq (selected-window) (frame-selected-window (selected-frame)))
>
> invariant?  That might be fatal.  Both, `with-selected-frame' and
> `with-selected-window', should leave no traces behind.
>
>  > Martin, that Qt in the Fselect_window call (the NORECORD argument) -
>  > would it be perhaps be better as Qnil?
>  >
>  >
>  > diff --git a/src/minibuf.c b/src/minibuf.c
>  > index cffb7fe787..3468643a7e 100644
>  > --- a/src/minibuf.c
>  > +++ b/src/minibuf.c
>  > @@ -893,6 +893,11 @@ read_minibuf (Lisp_Object map, Lisp_Object
> initial, Lisp_Object prompt,
>  >
>  >     run_hook (Qminibuffer_setup_hook);
>  >
>  > +  /* If the above hook has made the mini-window no longer the selected
>  > +     window, restore it.  */
>  > +  if (!EQ (selected_window, minibuf_window))
>  > +    Fselect_window (minibuf_window , Qt);
>  > +
>
> Are we sure that we want to disallow a function on
> `minibuffer-setup-hook' to change the selected window?  With Emacs 27
>
>
> (defun foo ()
>    (select-window (frame-first-window)))
>
> (add-hook 'minibuffer-setup-hook 'foo)
>
>
> works without any problems here.
>
> The NORECORD argument is important only if you need it - so far, the
> previous buffers of the minibuffer window were largely ignored.
>
> martin
>
>

[-- Attachment #2: Type: text/html, Size: 4257 bytes --]

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Martin.

On Thu, May 27, 2021 at 18:33:45 +0200, martin rudalics wrote:
>  > What is happening is that the with-selected-frame invocation is
>  > selecting (temporarily) a different frame from the minibuffer's
>  > frame.  This has the (intended) side effect of making the MB no
>  > longer selected in that frame.  When the MB's frame becomes selected
>  > again, nothing makes the mini-window the selected window.  This
>  > needs fixing.

> Does this mean that the

>    Fselect_window (f->selected_window, norecord);

> in do_switch_frame fails?  If so, why?

For some values of "fails", yes.  When we're in frame F1's minibuffer,
and do (with-selected-frame F2 (foo)):
(i) Frame F2 becomes selected.
(ii) The current window in F1 ceases to be the mini-window, becoming some
other window.
(iii) The minibuffer is moved from F1 to F2.

(iv) We evaluate (foo).

(v) Frame F1 becomes selected again.
(vi) The current window in F1 doesn't revert to being the mini-window.

> Do we anywhere violate the

>    (eq (selected-window) (frame-selected-window (selected-frame)))

> invariant?

No.  The variables selected-\(window\|frame\) are not set directly
anywhere in minibuf.c, only by calling functions like Fselect_window.

> That might be fatal.  Both, `with-selected-frame' and
> `with-selected-window', should leave no traces behind.

This is what has preoccupied me over the last few hours.  It seems we
want do_switch_frame to do different things for (i) a "permanent" frame
switch (e.g. C-x 5 o) and (i) a "temporary" frame switch (e.g.
with-selected-frame).  If the selected window in F1 is the mini-window,
we want it to select a different window in (i), but stay the same in
(ii). 

Or perhaps we want a variant of select-frame which wouldn't move a stack
of minibuffers from F1 to F2.  Something like Ftemporarily_select_frame,
which would come with discouragement from use in its doc string, and
wouldn't be interactive.  with-selected-frame would use this.  Something
like (untested):

DEFUN ("temporarily-select-frame", Ftemporarily_select_frame,
       Stemporarily_select_frame, 1, 1, null,
       doc: /* Temporarily select FRAME.

More doc string.......

This function returns FRAME, or nil if FRAME has been deleted.  */)
  (Lisp_Object frame)
{
  struct frame *f;

  CHECK_LIVE_FRAME (frame);
  f = XFRAME (frame);

  if (FRAME_TOOLTIP_P (f))
    /* Do not select a tooltip frame (Bug#47207).  */
    error ("Cannot select a tooltip frame");
  else
    return do_switch_frame (frame, 1, 0, Qt, 1); <==== Extra argument here.
}


(This is all with minibuffer-follows-selected-frame set to t.)

>  > Martin, that Qt in the Fselect_window call (the NORECORD argument) -
>  > would it be perhaps be better as Qnil?


>  > diff --git a/src/minibuf.c b/src/minibuf.c
>  > index cffb7fe787..3468643a7e 100644
>  > --- a/src/minibuf.c
>  > +++ b/src/minibuf.c
>  > @@ -893,6 +893,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
>  >
>  >     run_hook (Qminibuffer_setup_hook);
>  >
>  > +  /* If the above hook has made the mini-window no longer the selected
>  > +     window, restore it.  */
>  > +  if (!EQ (selected_window, minibuf_window))
>  > +    Fselect_window (minibuf_window , Qt);
>  > +

> Are we sure that we want to disallow a function on
> `minibuffer-setup-hook' to change the selected window?  With Emacs 27


> (defun foo ()
>    (select-window (frame-first-window)))

> (add-hook 'minibuffer-setup-hook 'foo)


> works without any problems here.

Are you sure?  For me, that setup makes C-x C-f open a minibuffer, but
leave point in the same window, not the miniwindow.  That was a quick try
with the emacs-27 branch, not Emacs 27.{1,2}, and was on a GUI.

I think we should disallow selecting windows in minibuffer-setup-hook.
This hook is run after the mini-window has been selected in read_minibuf,
just before the recursive edit.



> The NORECORD argument is important only if you need it - so far, the
> previous buffers of the minibuffer window were largely ignored.

Thanks.  I'll think about that when my head's a bit clearer.

> martin

bug#48674: Frames and minibuffer bug

[martin rudalics]

 >> The only difference is the add-hook, this time using
 >> window-state-change-hook instead of minibuffer-...
 >> This leads to the same bug.

It's still the same bug.  No matter what happens, evaluating

(let ((window (selected-window)))
   (dolist (frame (frame-list))
     (with-selected-frame frame))
   (eq window (selected-window)))

must always yield t.

Note, however, that the `with-selected-frame' in your function is not
needed.  `modify-frame-parameters' works without selecting the frame
whose parameter you want to modify.  Also I do not understand why you
want to modify the cursor in all windows when you enter the minibuffer.

Consider the following snippet to set the background of the minibuffer
window when it is active (hopefully `active-minibuffer-window' always
returns the right value when exiting the minibuffer).


(defun foo ()
   (with-current-buffer (window-buffer (active-minibuffer-window))
     (set (make-local-variable 'face-remapping-alist)
	 '((default (:background "yellow") default)))))

(defun bar ()
   (with-current-buffer (window-buffer (active-minibuffer-window))
     (set (make-local-variable 'face-remapping-alist) nil)))

(add-hook 'minibuffer-setup-hook #'foo)
(add-hook 'minibuffer-exit-hook #'bar)


I never tried to remap the cursor color in a buffer-local way, but I
think you can figure out how to do that in a similar fashion.

martin

bug#48674: Frames and minibuffer bug

[martin rudalics]

 >> Does this mean that the
 >
 >>     Fselect_window (f->selected_window, norecord);
 >
 >> in do_switch_frame fails?  If so, why?
 >
 > For some values of "fails", yes.  When we're in frame F1's minibuffer,
 > and do (with-selected-frame F2 (foo)):
 > (i) Frame F2 becomes selected.
 > (ii) The current window in F1 ceases to be the mini-window, becoming some
 > other window.
 > (iii) The minibuffer is moved from F1 to F2.
 >
 > (iv) We evaluate (foo).
 >
 > (v) Frame F1 becomes selected again.
 > (vi) The current window in F1 doesn't revert to being the mini-window.

But why precisely does Fselect_window fail here?

 > This is what has preoccupied me over the last few hours.  It seems we
 > want do_switch_frame to do different things for (i) a "permanent" frame
 > switch (e.g. C-x 5 o) and (i) a "temporary" frame switch (e.g.
 > with-selected-frame).  If the selected window in F1 is the mini-window,
 > we want it to select a different window in (i), but stay the same in
 > (ii).

Be careful in one additional regard: The display engine calls
Fselect_window too for its own purposes.

 >> Are we sure that we want to disallow a function on
 >> `minibuffer-setup-hook' to change the selected window?  With Emacs 27
 >
 >
 >> (defun foo ()
 >>     (select-window (frame-first-window)))
 >
 >> (add-hook 'minibuffer-setup-hook 'foo)
 >
 >
 >> works without any problems here.
 >
 > Are you sure?  For me, that setup makes C-x C-f open a minibuffer, but
 > leave point in the same window, not the miniwindow.  That was a quick try
 > with the emacs-27 branch, not Emacs 27.{1,2}, and was on a GUI.

Yes.  But this is an effect the user/application might want - explicitly
select some other window to, for example (I'm purely speculating), select
a completion window right away.  Iris OTOH never wants to select another
window explicitly.

 > I think we should disallow selecting windows in minibuffer-setup-hook.
 > This hook is run after the mini-window has been selected in read_minibuf,
 > just before the recursive edit.

We cannot disallow it.  We can only undo its effect.  And that would
constitute an incompatible change - do you really want to go for it?

martin

bug#48674: Frames and minibuffer bug

[Iris García]

[-- Attachment #1: Type: text/plain, Size: 2432 bytes --]

Hi Martin,

Thanks for the comments, to be honest, my elisp knowledge is very limited
and I wanted to learn by doing a modal editing package.

This package has two modes: normal and insert, I like to change the cursor
color and type according to the active editing mode.

For this reason, I have a hook in the minibuffer, so everytime I open the
minibuffer I want to switch to insert mode and this triggers the
cursor update in every frame then when the minibufer is closed go back to
normal mode and again update the cursor in every frame.

I'm pretty sure there are better ways to achieve this, but this was the one
I got working in Emacs 27.

I'm happy to share the entire code if that helps but I think the point here
is the bug is real and should be fixed no?

In the meantime, I'll take your snippet (thanks for that) and try to edit
my package to workaround the bug.

Regards,

Iris.

On Fri, 28 May 2021 at 10:25, martin rudalics <rudalics@gmx.at> wrote:

>  >> The only difference is the add-hook, this time using
>  >> window-state-change-hook instead of minibuffer-...
>  >> This leads to the same bug.
>
> It's still the same bug.  No matter what happens, evaluating
>
> (let ((window (selected-window)))
>    (dolist (frame (frame-list))
>      (with-selected-frame frame))
>    (eq window (selected-window)))
>
> must always yield t.
>
> Note, however, that the `with-selected-frame' in your function is not
> needed.  `modify-frame-parameters' works without selecting the frame
> whose parameter you want to modify.  Also I do not understand why you
> want to modify the cursor in all windows when you enter the minibuffer.
>
> Consider the following snippet to set the background of the minibuffer
> window when it is active (hopefully `active-minibuffer-window' always
> returns the right value when exiting the minibuffer).
>
>
> (defun foo ()
>    (with-current-buffer (window-buffer (active-minibuffer-window))
>      (set (make-local-variable 'face-remapping-alist)
>          '((default (:background "yellow") default)))))
>
> (defun bar ()
>    (with-current-buffer (window-buffer (active-minibuffer-window))
>      (set (make-local-variable 'face-remapping-alist) nil)))
>
> (add-hook 'minibuffer-setup-hook #'foo)
> (add-hook 'minibuffer-exit-hook #'bar)
>
>
> I never tried to remap the cursor color in a buffer-local way, but I
> think you can figure out how to do that in a similar fashion.
>
> martin
>

[-- Attachment #2: Type: text/html, Size: 3166 bytes --]

bug#48674: Frames and minibuffer bug

[martin rudalics]

 > I'm happy to share the entire code if that helps but I think the point here
 > is the bug is real and should be fixed no?

The bug is real and should be fixed.

martin

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Martin.

On Fri, May 28, 2021 at 10:26:32 +0200, martin rudalics wrote:
>  >> Does this mean that the

>  >>     Fselect_window (f->selected_window, norecord);

>  >> in do_switch_frame fails?  If so, why?

>  > For some values of "fails", yes.  When we're in frame F1's minibuffer,
>  > and do (with-selected-frame F2 (foo)):
>  > (i) Frame F2 becomes selected.
>  > (ii) The current window in F1 ceases to be the mini-window, becoming some
>  > other window.
>  > (iii) The minibuffer is moved from F1 to F2.

>  > (iv) We evaluate (foo).

>  > (v) Frame F1 becomes selected again.
>  > (vi) The current window in F1 doesn't revert to being the mini-window.

> But why precisely does Fselect_window fail here?

Sorry, my last night's reply wasn't very clear.  That Fselect_window in
do_switch_frame will fail when a later Fselect_window (or more precisely,
Fset_frame_selected_window) in move_minibuffers_onto_frame selects a
different window for that frame.  That happens when with-selected-frame
selects a different frame, moving the minibuffers onto that other frame.

I have another idea for solving this problem.  Suppose when we switch
from F1 to F2, and F1's selected window is the mini-window, we allow F1
to remain in the MW.  The minibuffer stack gets moved to F2.  Some sort
of commands or Lisp runs in F2, then we return to F1.  Should any
minibuffer come back to F1, we allow the MW to remain selected.
Otherwise we select a different window in F1.

I think this would work better that what I proposed last night.  I'll try
and formulate a patch for this this evening.

>  > This is what has preoccupied me over the last few hours.  It seems
>  > we want do_switch_frame to do different things for (i) a "permanent"
>  > frame switch (e.g. C-x 5 o) and (i) a "temporary" frame switch (e.g.
>  > with-selected-frame).  If the selected window in F1 is the
>  > mini-window, we want it to select a different window in (i), but
>  > stay the same in (ii).

> Be careful in one additional regard: The display engine calls
> Fselect_window too for its own purposes.

.... and Fselect_window calls Fselect_frame, if needed.  This hits the
same problem.

>  >> Are we sure that we want to disallow a function on
>  >> `minibuffer-setup-hook' to change the selected window?  With Emacs 27


>  >> (defun foo ()
>  >>     (select-window (frame-first-window)))

>  >> (add-hook 'minibuffer-setup-hook 'foo)


>  >> works without any problems here.

>  > Are you sure?  For me, that setup makes C-x C-f open a minibuffer,
>  > but leave point in the same window, not the miniwindow.  That was a
>  > quick try with the emacs-27 branch, not Emacs 27.{1,2}, and was on a
>  > GUI.

> Yes.  But this is an effect the user/application might want - explicitly
> select some other window to, for example (I'm purely speculating), select
> a completion window right away.  Iris OTOH never wants to select another
> window explicitly.

OK.

>  > I think we should disallow selecting windows in minibuffer-setup-hook.
>  > This hook is run after the mini-window has been selected in read_minibuf,
>  > just before the recursive edit.

> We cannot disallow it.  We can only undo its effect.  And that would
> constitute an incompatible change - do you really want to go for it?

Maybe not.  I'm going to write a patch as outlined above, which might
well be easier to follow than my text above.  I think it might well fix
the bug properly.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Martin.

On Fri, May 28, 2021 at 17:15:37 +0000, Alan Mackenzie wrote:
> On Fri, May 28, 2021 at 10:26:32 +0200, martin rudalics wrote:

[ .... ]

> > But why precisely does Fselect_window fail here?

> Sorry, my last night's reply wasn't very clear.  That Fselect_window in
> do_switch_frame will fail when a later Fselect_window (or more precisely,
> Fset_frame_selected_window) in move_minibuffers_onto_frame selects a
> different window for that frame.  That happens when with-selected-frame
> selects a different frame, moving the minibuffers onto that other frame.

> I have another idea for solving this problem.  Suppose when we switch
> from F1 to F2, and F1's selected window is the mini-window, we allow F1
> to remain in the MW.  The minibuffer stack gets moved to F2.  Some sort
> of commands or Lisp runs in F2, then we return to F1.  Should any
> minibuffer come back to F1, we allow the MW to remain selected.
> Otherwise we select a different window in F1.

> I think this would work better that what I proposed last night.  I'll try
> and formulate a patch for this this evening.

I think the following patch, along the above lines, solves the bug
completely.  Could you review it for me, please?



diff --git a/src/minibuf.c b/src/minibuf.c
index cffb7fe787..c97a672ad6 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -192,10 +192,10 @@ move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
   struct frame *f = XFRAME (selected_frame);
 
   minibuf_window = f->minibuffer_window;
-  if (!(minibuf_level
-	&& (for_deletion || minibuf_follows_frame () || FRAME_INITIAL_P (of))))
+  if (!(for_deletion || minibuf_follows_frame () || FRAME_INITIAL_P (of)))
     return;
-  if (FRAME_LIVE_P (f)
+  if (minibuf_level
+      && FRAME_LIVE_P (f)
       && !EQ (f->minibuffer_window, of->minibuffer_window)
       && WINDOW_LIVE_P (f->minibuffer_window) /* F not a tootip frame */
       && WINDOW_LIVE_P (of->minibuffer_window))
@@ -203,15 +203,12 @@ move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
       zip_minibuffer_stacks (f->minibuffer_window, of->minibuffer_window);
       if (for_deletion && XFRAME (MB_frame) != of)
 	MB_frame = selected_frame;
-      if (!for_deletion
-	  && MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-	{
-	  Lisp_Object old_frame;
-	  XSETFRAME (old_frame, of);
-	  Fset_frame_selected_window (old_frame,
-				      Fframe_first_window (old_frame), Qnil);
-	}
     }
+  /* If the new frame's selected window is the mini-window, select
+     some other window if we don't have an active minibuffer there.  */
+  if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (f)))
+      && !live_minibuffer_p (XWINDOW (FRAME_SELECTED_WINDOW (f))->contents))
+    Fselect_window (Fframe_first_window (selected_frame), Qnil);
 }
 
 DEFUN ("active-minibuffer-window", Factive_minibuffer_window,


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[martin rudalics]

 > I think the following patch, along the above lines, solves the bug
 > completely.  Could you review it for me, please?

It fixes the bug and the potential use case I mentioned earlier.  But
for this part

   /* If the new frame's selected window is the mini-window, select
      some other window if we don't have an active minibuffer there.  */
   if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (f)))
       && !live_minibuffer_p (XWINDOW (FRAME_SELECTED_WINDOW (f))->contents))
     Fselect_window (Fframe_first_window (selected_frame), Qnil);

I would first have to understand why such a minibuffer window should be
OT1H its frame's selected window and OTOH not be active when switching
back to that frame.  As a user I can always do

(select-window (minibuffer-window))

regardless of whether a minibuffer is active.  Switching away from that
window's frame and back to it is problematic in Emacs 27 - no window has
the active cursor.  You apparently fixed this by selecting the frame's
first window when switching back.  Right?

One reason why I ask is because with emacs -Q and the present patch

(setq default-frame-alist '((minibuffer . nil)))

followed by C-x 5 2 and

(select-window (minibuffer-window))

in the new frame violates the assertion on line 548 of window.c

       /* Fselect_frame called us back so we've done all the work already.  */
       eassert (EQ (window, selected_window));

with the backtrace below.

Basically, I would try to avoid that a non-selected frame's selected
window is an inactive minibuffer window.  But I'm not sure how difficult
it is to achieve that.

martin


#0  0x00000000005ac2a7 in terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../src/emacs.c:400
#1  0x00000000006593a5 in die (msg=0x792187 "EQ (window, selected_window)", file=0x791f12 "../../src/window.c", line=548) at ../../src/alloc.c:7451
#2  0x00000000004c8185 in select_window (window=XIL(0x11109dd), norecord=XIL(0), inhibit_point_swap=false) at ../../src/window.c:548
#3  0x00000000004c8315 in Fselect_window (window=XIL(0x11109dd), norecord=XIL(0)) at ../../src/window.c:625
#4  0x000000000069110e in eval_sub (form=XIL(0x1102163)) at ../../src/eval.c:2517
#5  0x000000000068af94 in Fprogn (body=XIL(0)) at ../../src/eval.c:471
#6  0x0000000000690cfc in eval_sub (form=XIL(0x1102123)) at ../../src/eval.c:2467
#7  0x0000000000690661 in Feval (form=XIL(0x1102123), lexical=XIL(0x30)) at ../../src/eval.c:2343
#8  0x0000000000692dc5 in funcall_subr (subr=0xc61600 <Seval>, numargs=2, args=0x7fffffffce80) at ../../src/eval.c:3116
#9  0x000000000069285e in Ffuncall (nargs=3, args=0x7fffffffce78) at ../../src/eval.c:3039
#10 0x00000000006ec8e5 in exec_byte_code (bytestr=XIL(0x7ffff3e1e10c), vector=XIL(0x7ffff3e1ccd5), maxdepth=make_fixnum(18), args_template=make_fixnum(257), nargs=1, args=0x7fffffffd380) at ../../src/bytecode.c:632
#11 0x0000000000693026 in fetch_and_exec_byte_code (fun=XIL(0x7ffff3e1cca5), syms_left=make_fixnum(257), nargs=1, args=0x7fffffffd378) at ../../src/eval.c:3163
#12 0x00000000006934ac in funcall_lambda (fun=XIL(0x7ffff3e1cca5), nargs=1, arg_vector=0x7fffffffd378) at ../../src/eval.c:3244
#13 0x00000000006928b2 in Ffuncall (nargs=2, args=0x7fffffffd370) at ../../src/eval.c:3043
#14 0x00000000006ec8e5 in exec_byte_code (bytestr=XIL(0x7ffff3e1e38c), vector=XIL(0x7ffff3e1cc45), maxdepth=make_fixnum(4), args_template=make_fixnum(257), nargs=1, args=0x7fffffffd978) at ../../src/bytecode.c:632
#15 0x0000000000693026 in fetch_and_exec_byte_code (fun=XIL(0x7ffff3e1cc0d), syms_left=make_fixnum(257), nargs=1, args=0x7fffffffd970) at ../../src/eval.c:3163
#16 0x00000000006934ac in funcall_lambda (fun=XIL(0x7ffff3e1cc0d), nargs=1, arg_vector=0x7fffffffd970) at ../../src/eval.c:3244
#17 0x00000000006928b2 in Ffuncall (nargs=2, args=0x7fffffffd968) at ../../src/eval.c:3043
#18 0x000000000068656e in Ffuncall_interactively (nargs=2, args=0x7fffffffd968) at ../../src/callint.c:260
#19 0x0000000000692c98 in funcall_subr (subr=0xc60e40 <Sfuncall_interactively>, numargs=2, args=0x7fffffffd968) at ../../src/eval.c:3094
#20 0x000000000069285e in Ffuncall (nargs=3, args=0x7fffffffd960) at ../../src/eval.c:3039
#21 0x0000000000688bf4 in Fcall_interactively (function=XIL(0x7ffff3134918), record_flag=XIL(0), keys=XIL(0x7ffff4375585)) at ../../src/callint.c:791
#22 0x0000000000692df1 in funcall_subr (subr=0xc60e80 <Scall_interactively>, numargs=3, args=0x7fffffffdd40) at ../../src/eval.c:3119
#23 0x000000000069285e in Ffuncall (nargs=4, args=0x7fffffffdd38) at ../../src/eval.c:3039
#24 0x00000000006ec8e5 in exec_byte_code (bytestr=XIL(0x7ffff3db0ccc), vector=XIL(0x7ffff3db0935), maxdepth=make_fixnum(13), args_template=make_fixnum(1025), nargs=1, args=0x7fffffffe2b0) at ../../src/bytecode.c:632
#25 0x0000000000693026 in fetch_and_exec_byte_code (fun=XIL(0x7ffff3db0905), syms_left=make_fixnum(1025), nargs=1, args=0x7fffffffe2a8) at ../../src/eval.c:3163
#26 0x00000000006934ac in funcall_lambda (fun=XIL(0x7ffff3db0905), nargs=1, arg_vector=0x7fffffffe2a8) at ../../src/eval.c:3244
#27 0x00000000006928b2 in Ffuncall (nargs=2, args=0x7fffffffe2a0) at ../../src/eval.c:3043
#28 0x000000000069217b in call1 (fn=XIL(0x43e0), arg1=XIL(0x7ffff3134918)) at ../../src/eval.c:2899
#29 0x00000000005b4838 in command_loop_1 () at ../../src/keyboard.c:1466
#30 0x000000000068e3bd in internal_condition_case (bfun=0x5b3fdf <command_loop_1>, handlers=XIL(0x90), hfun=0x5b35ee <cmd_error>) at ../../src/eval.c:1478
#31 0x00000000005b3bc4 in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1094
#32 0x000000000068d59a in internal_catch (tag=XIL(0xe100), func=0x5b3b97 <command_loop_2>, arg=XIL(0)) at ../../src/eval.c:1198
#33 0x00000000005b3b62 in command_loop () at ../../src/keyboard.c:1073
#34 0x00000000005b30d5 in recursive_edit_1 () at ../../src/keyboard.c:720
#35 0x00000000005b32cd in Frecursive_edit () at ../../src/keyboard.c:789
#36 0x00000000005af1a4 in main (argc=2, argv=0x7fffffffe7c8) at ../../src/emacs.c:2298

Lisp Backtrace:
"select-window" (0xffffcaa0)
"progn" (0xffffcc50)
"eval" (0xffffce80)
"elisp--eval-last-sexp" (0xffffd378)
"eval-last-sexp" (0xffffd970)
"funcall-interactively" (0xffffd968)
"call-interactively" (0xffffdd40)
"command-execute" (0xffffe2a8)
(gdb)

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Martin.

On Sat, May 29, 2021 at 11:20:58 +0200, martin rudalics wrote:
>  > I think the following patch, along the above lines, solves the bug
>  > completely.  Could you review it for me, please?

Thanks for the review.  I withdraw my assertion about a complete
solution to the bug.  :-(

> It fixes the bug and the potential use case I mentioned earlier.  But
> for this part

>    /* If the new frame's selected window is the mini-window, select
>       some other window if we don't have an active minibuffer there.  */
>    if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (f)))
>        && !live_minibuffer_p (XWINDOW (FRAME_SELECTED_WINDOW (f))->contents))
>      Fselect_window (Fframe_first_window (selected_frame), Qnil);

> I would first have to understand why such a minibuffer window should be
> OT1H its frame's selected window and OTOH not be active when switching
> back to that frame.

When the user switches from F1 to F2 with C-x 5 o, and there was an
active minibuffer on F1, that MB gets moved to F2.  The user then
terminnates the MB....then switches back to F1.  There is no longer an
active MB.

> As a user I can always do

> (select-window (minibuffer-window))

> regardless of whether a minibuffer is active.  Switching away from that
> window's frame and back to it is problematic in Emacs 27 - no window has
> the active cursor.  You apparently fixed this by selecting the frame's
> first window when switching back.  Right?

Yes.

> One reason why I ask is because with emacs -Q and the present patch

> (setq default-frame-alist '((minibuffer . nil)))

> followed by C-x 5 2 and

> (select-window (minibuffer-window))

> in the new frame violates the assertion on line 548 of window.c

>        /* Fselect_frame called us back so we've done all the work already.  */
>        eassert (EQ (window, selected_window));

> with the backtrace below.

Just as a quick aside, how do I configure Emacs so as to generate these
easserts?  At the moment I've got CFLAGS='-O0 -g3' and --enable-checking
in my ./configure command line.  I think I'm missing something.

> Basically, I would try to avoid that a non-selected frame's selected
> window is an inactive minibuffer window.  But I'm not sure how difficult
> it is to achieve that.

That's the way it currently is (without the patch).  It is difficult to
make it work properly.

Maybe the best workaround would be to create a new flag in struct frame,
which when set means "select mini-window if "possible" when selecting
this frame".  "Possible" meaning there's an active MB.
(i) This flag would be set, somehow, by with-selected-frame when moving
away from F1.  
(ii) do_switch_frame would test this flag, act on it, and clear it.
f->flag would always be clear when f is the selected frame.

With this, F1's selected window would be moved away from the mini-window
and the flag set, when F2 gets selected.  Or something like that.

What do you think?

> martin

[ Backtrace snipped, but understood. ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[martin rudalics]

 > When the user switches from F1 to F2 with C-x 5 o, and there was an
 > active minibuffer on F1, that MB gets moved to F2.  The user then
 > terminnates the MB....then switches back to F1.  There is no longer an
 > active MB.
 >
 >> As a user I can always do
 >
 >> (select-window (minibuffer-window))
 >
 >> regardless of whether a minibuffer is active.  Switching away from that
 >> window's frame and back to it is problematic in Emacs 27 - no window has
 >> the active cursor.  You apparently fixed this by selecting the frame's
 >> first window when switching back.  Right?
 >
 > Yes.

This is safe but not quite right.  The last window on that frame
selected before the minibuffer window got selected should be selected
instead - that's the window returned by `minibuffer-selected-window'
immediately after selecting the minibuffer window.  We'd probably have
to put that into the frame structure to make it work but this can wait.

 >> One reason why I ask is because with emacs -Q and the present patch
 >
 >> (setq default-frame-alist '((minibuffer . nil)))
 >
 >> followed by C-x 5 2 and
 >
 >> (select-window (minibuffer-window))
 >
 >> in the new frame violates the assertion on line 548 of window.c
 >
 >>         /* Fselect_frame called us back so we've done all the work already.  */
 >>         eassert (EQ (window, selected_window));
 >
 >> with the backtrace below.
 >
 > Just as a quick aside, how do I configure Emacs so as to generate these
 > easserts?  At the moment I've got CFLAGS='-O0 -g3' and --enable-checking

I use

  --enable-checking='yes,glyphs' --enable-check-lisp-object-type=yes

but cannot tell which ones are right, which matter and from where I
obtained them.

 > in my ./configure command line.  I think I'm missing something.
 >
 >> Basically, I would try to avoid that a non-selected frame's selected
 >> window is an inactive minibuffer window.  But I'm not sure how difficult
 >> it is to achieve that.
 >
 > That's the way it currently is (without the patch).  It is difficult to
 > make it work properly.
 >
 > Maybe the best workaround would be to create a new flag in struct frame,
 > which when set means "select mini-window if "possible" when selecting
 > this frame".  "Possible" meaning there's an active MB.
 > (i) This flag would be set, somehow, by with-selected-frame when moving
 > away from F1.

I doubt that anyone knows when we move away from a frame.  With

emacs -Q --eval "(setq default-frame-alist '((minibuffer)))"

do in the normal, non-minibuffer frame

(select-window (minibuffer-window))

You will see that the previous window remains selected but its mode line
changes to inactive.  So even the display engine doesn't know which
frame is selected here.  Maybe on purpose ...

 > (ii) do_switch_frame would test this flag, act on it, and clear it.
 > f->flag would always be clear when f is the selected frame.
 >
 > With this, F1's selected window would be moved away from the mini-window
 > and the flag set, when F2 gets selected.  Or something like that.

do_switch_frame is called by (i) and (ii) - so how would you "set" this
flag in (i) and "test this flag and act on it" in (ii)?  How would
do_switch_frame distinguish between (i) and (ii)?

OTOH what would go wrong if we did always auto-select a minibuffer
window in do_switch_frame when it is its `frame-selected-window' and
`minibuffer-depth' is non-zero regardless of whether the window's
minibuffer is on live_minibuffer_p or not?  If it is not, then this
frame would just become the next participant in moving the minibuffer
from one frame to another.

martin

bug#48674: Frames and minibuffer bug

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 29 May 2021 17:12:58 +0200
> Cc: 48674@debbugs.gnu.org,
>  Iris García <iris.garcia.desebastian@gmail.com>
> 
>  > Just as a quick aside, how do I configure Emacs so as to generate these
>  > easserts?  At the moment I've got CFLAGS='-O0 -g3' and --enable-checking
> 
> I use
> 
>   --enable-checking='yes,glyphs' --enable-check-lisp-object-type=yes
> 
> but cannot tell which ones are right, which matter and from where I
> obtained them.

Assertions via eassert are activated by --enable-checking.

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Martin.

On Sat, May 29, 2021 at 17:12:58 +0200, martin rudalics wrote:
>  > When the user switches from F1 to F2 with C-x 5 o, and there was an
>  > active minibuffer on F1, that MB gets moved to F2.  The user then
>  > terminnates the MB....then switches back to F1.  There is no longer an
>  > active MB.

>  >> As a user I can always do

>  >> (select-window (minibuffer-window))

>  >> regardless of whether a minibuffer is active.  Switching away from that
>  >> window's frame and back to it is problematic in Emacs 27 - no window has
>  >> the active cursor.  You apparently fixed this by selecting the frame's
>  >> first window when switching back.  Right?

>  > Yes.

> This is safe but not quite right.  The last window on that frame
> selected before the minibuffer window got selected should be selected
> instead - that's the window returned by `minibuffer-selected-window'
> immediately after selecting the minibuffer window.  We'd probably have
> to put that into the frame structure to make it work but this can wait.

I agree, this can wait.  ;-)

>  >> One reason why I ask is because with emacs -Q and the present patch

>  >> (setq default-frame-alist '((minibuffer . nil)))

>  >> followed by C-x 5 2 and

>  >> (select-window (minibuffer-window))

>  >> in the new frame violates the assertion on line 548 of window.c

>  >>         /* Fselect_frame called us back so we've done all the work already.  */
>  >>         eassert (EQ (window, selected_window));

>  >> with the backtrace below.

>  > Just as a quick aside, how do I configure Emacs so as to generate these
>  > easserts?  At the moment I've got CFLAGS='-O0 -g3' and --enable-checking

> I use

>   --enable-checking='yes,glyphs' --enable-check-lisp-object-type=yes

> but cannot tell which ones are right, which matter and from where I
> obtained them.

Thanks.  It was just me being a bit stupid.  I had
minibuffer-follows-selected-frame set to nil, so I didn't get to the code
which easserts.  On setting that variable to t, I did get it.

[ .... ]

>  > Maybe the best workaround would be to create a new flag in struct frame,
>  > which when set means "select mini-window if "possible" when selecting
>  > this frame".  "Possible" meaning there's an active MB.
>  > (i) This flag would be set, somehow, by with-selected-frame when moving
>  > away from F1.

> I doubt that anyone knows when we move away from a frame.  With

> emacs -Q --eval "(setq default-frame-alist '((minibuffer)))"

> do in the normal, non-minibuffer frame

> (select-window (minibuffer-window))

> You will see that the previous window remains selected but its mode line
> changes to inactive.  So even the display engine doesn't know which
> frame is selected here.  Maybe on purpose ...

I've been thinking about my proposed mechanism.  There is no need for
with-selected-frame to set that new flag in struct frame.  More simply,
do_switch_frame can set it whenever the old frame has the mini-window
selected.  In the new frame it will check the flag and if appropriate
select the MW.  Functions such as Fselect_window and
Fset_frame_selected_window which want to select a specific window would
clear the flag.

This mechanism could be implemented entirely in the C code, without
affecting any Lisp interfaces.

>  > (ii) do_switch_frame would test this flag, act on it, and clear it.
>  > f->flag would always be clear when f is the selected frame.

>  > With this, F1's selected window would be moved away from the mini-window
>  > and the flag set, when F2 gets selected.  Or something like that.

> do_switch_frame is called by (i) and (ii) - so how would you "set" this
> flag in (i) and "test this flag and act on it" in (ii)?  How would
> do_switch_frame distinguish between (i) and (ii)?

Sorry, I think I was a bit unclear.  The flag would be in struct frame,
so (i) and (ii) would be acting on the flags for different frames.

> OTOH what would go wrong if we did always auto-select a minibuffer
> window in do_switch_frame when it is its `frame-selected-window' and
> `minibuffer-depth' is non-zero regardless of whether the window's
> minibuffer is on live_minibuffer_p or not?  If it is not, then this
> frame would just become the next participant in moving the minibuffer
> from one frame to another.

I don't think that situation can arise.  We're talking exclusively about
the (eq minibuffer-follows-selected-frame t) case, and in this case there
will always be a live minibuffer moved onto the target frame when
minibuffer-depth is non-zero.

I think I'm going to discard yesterday's attempted patch, and write
another patch along the lines sketched above.  This will likely take me
longer than just this evening.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello again, Martin.

On Sat, May 29, 2021 at 17:00:04 +0000, Alan Mackenzie wrote:
> On Sat, May 29, 2021 at 17:12:58 +0200, martin rudalics wrote:

[ .... ]

> >  > Maybe the best workaround would be to create a new flag in struct
> >  > frame, which when set means "select mini-window if "possible"
> >  > when selecting this frame".  "Possible" meaning there's an active
> >  > MB.  (i) This flag would be set, somehow, by with-selected-frame
> >  > when moving away from F1.

[ .... ]

> I've been thinking about my proposed mechanism.  There is no need for
> with-selected-frame to set that new flag in struct frame.  More simply,
> do_switch_frame can set it whenever the old frame has the mini-window
> selected.  In the new frame it will check the flag and if appropriate
> select the MW.  Functions such as Fselect_window and
> Fset_frame_selected_window which want to select a specific window would
> clear the flag.

> This mechanism could be implemented entirely in the C code, without
> affecting any Lisp interfaces.

[ .... ]

> I think I'm going to discard yesterday's attempted patch, and write
> another patch along the lines sketched above.  This will likely take me
> longer than just this evening.

Here is that patch.  Please tell me what you think of it.  Thanks!



diff --git a/src/frame.c b/src/frame.c
index e3d65dd28f..623e4ba2cd 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -982,6 +982,7 @@ make_frame (bool mini_p)
   f->ns_transparent_titlebar = false;
 #endif
 #endif
+  f->select_mini_window_flag = false;
   /* This one should never be zero.  */
   f->change_stamp = 1;
   root_window = make_window ();
@@ -1542,7 +1543,17 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
       tty->top_frame = frame;
     }
 
+  sf->select_mini_window_flag = MINI_WINDOW_P (XWINDOW (sf->selected_window));
+
   selected_frame = frame;
+
+  move_minibuffers_onto_frame (sf, for_deletion);
+
+  if (f->select_mini_window_flag
+      && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
+    f->selected_window = f->minibuffer_window;
+  f->select_mini_window_flag = false;
+
   if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
     last_nonminibuf_frame = XFRAME (selected_frame);
 
@@ -1559,7 +1570,6 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
 #endif
     internal_last_event_frame = Qnil;
 
-  move_minibuffers_onto_frame (sf, for_deletion);
   return frame;
 }
 
diff --git a/src/frame.h b/src/frame.h
index 75a0b184c1..cad3df5ae1 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -462,6 +462,11 @@ struct frame
      in X builds only.  */
   bool_bf was_invisible : 1;
 
+  /* True when the frame isn't selected, and selecting it in the
+     future should select the mini-window rather than the currently
+     selected window in the frame, assuming there is still an active
+     minibuffer in that mini-window.  */
+  bool_bf select_mini_window_flag : 1;
   /* Bitfield area ends here.  */
 
   /* This frame's change stamp, set the last time window change
diff --git a/src/window.c b/src/window.c
index 9961c54161..f0451359c1 100644
--- a/src/window.c
+++ b/src/window.c
@@ -468,6 +468,7 @@ Return WINDOW.  */)
   else
     {
       fset_selected_window (XFRAME (frame), window);
+      /* Don't clear FRAME's select_mini_window_flag here.  */
       return window;
     }
 }
@@ -517,6 +518,9 @@ select_window (Lisp_Object window, Lisp_Object norecord,
     /* Do not select a tooltip window (Bug#47207).  */
     error ("Cannot select a tooltip window");
 
+  /* We deinitely want to select WINDOW, not the mini-window.  */
+  f->select_mini_window_flag = false;
+
   /* Make the selected window's buffer current.  */
   Fset_buffer (w->contents);
 
@@ -3242,6 +3246,9 @@ window-start value is reasonable when this function is called.  */)
 	  if (EQ (selected_frame, w->frame))
 	    Fselect_window (window, Qnil);
 	  else
+	    /* Do not clear f->select_mini_window_flag here.  If the
+	       last selected window on F was an active minibuffer, we
+	       want to return to it on a later Fselect_frame.  */
 	    fset_selected_window (f, window);
 	}
     }
@@ -5153,7 +5160,10 @@ Signal an error when WINDOW is the only window on its frame.  */)
 	  if (EQ (FRAME_SELECTED_WINDOW (f), selected_window))
 	    Fselect_window (new_selected_window, Qt);
 	  else
-	    fset_selected_window (f, new_selected_window);
+	    /* Do not clear f->select_mini_window_flag here.  If the
+	       last selected window on F was an active minibuffer, we
+	       want to return to it on a later Fselect_frame.  */
+	      fset_selected_window (f, new_selected_window);
 
 	  unblock_input ();
 


> > martin

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#48674: Frames and minibuffer bug

[martin rudalics]

 > Here is that patch.  Please tell me what you think of it.  Thanks!

It seems to work so please install.

Thanks, martin

bug#48674: Frames and minibuffer bug

[Alan Mackenzie]

Hello, Iris.

Firstly, forgive me for not answering you sooner.  I didn't want want to
waste any more of your time with any more unusable patches.  This was a
tricky bug to solve, and indeed only the third patch attempt was
satisfactory.

I have now committed this third patch, and would ask you to remove the
patch I sent you a few days ago, and update your Emacs to the current
master version.

I am closing the bug with this post, but if you find any more trouble
with it, would you please let us know, so that we can open it again.
Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).


On Thu, May 27, 2021 at 19:56:03 +0000, Iris García wrote:
> Hi Martin,

> I forgot to include you in my last mail where I said:

> I think I have found the new issue (it is related to the former one), my
> > code this time was the following:

> > (defvar box-cursor t)
> >
> > (defun test/set-cursor()
> >   "Set cursor in all frames depending on the active state."
> >   (interactive)
> >   (dolist (frame (frame-list))
> >     (with-selected-frame frame
> >       (if box-cursor
> >           (progn
> >             (modify-frame-parameters
> >              frame (list (cons 'cursor-type 'box)))
> >             (modify-frame-parameters
> >              frame (list (cons 'cursor-color "#00A9FE"))))
> >         (progn
> >           (modify-frame-parameters
> >            frame (list (cons 'cursor-type 'hbar)))
> >           (modify-frame-parameters
> >            frame (list (cons 'cursor-color "green")))
> >           )))))
> >
> > (defun test/enter-minibuffer()
> >   (setq box-cursor nil)
> >   (test/set-cursor))
> >
> > (defun test/exit-minibuffer()
> >   (setq box-cursor t)
> >   (test/set-cursor))
> >
> >
> > (add-hook 'window-state-change-hook #'test/enter-minibuffer)
> > (add-hook 'window-state-change-hook #'test/exit-minibuffer)
> >
> > (server-start)
> > (make-frame

> > The only difference is the add-hook, this time using
> > window-state-change-hook instead of minibuffer-...
> > This leads to the same bug.

> > Regards,

> > Iris.

bug#48674: Frames and minibuffer bug

[Iris García]

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]

Hi Alan,

No worries! I did follow the thread and tried your 3 patches, the last one
is indeed working as expected as far as I can tell.

Thank you very much for the quick response and fix.

Regards,

Iris.

On Mon, 31 May 2021 at 18:36, Alan Mackenzie <acm@muc.de> wrote:

> Hello, Iris.
>
> Firstly, forgive me for not answering you sooner.  I didn't want want to
> waste any more of your time with any more unusable patches.  This was a
> tricky bug to solve, and indeed only the third patch attempt was
> satisfactory.
>
> I have now committed this third patch, and would ask you to remove the
> patch I sent you a few days ago, and update your Emacs to the current
> master version.
>
> I am closing the bug with this post, but if you find any more trouble
> with it, would you please let us know, so that we can open it again.
> Thanks!
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>
>
> On Thu, May 27, 2021 at 19:56:03 +0000, Iris García wrote:
> > Hi Martin,
>
> > I forgot to include you in my last mail where I said:
>
> > I think I have found the new issue (it is related to the former one), my
> > > code this time was the following:
>
> > > (defvar box-cursor t)
> > >
> > > (defun test/set-cursor()
> > >   "Set cursor in all frames depending on the active state."
> > >   (interactive)
> > >   (dolist (frame (frame-list))
> > >     (with-selected-frame frame
> > >       (if box-cursor
> > >           (progn
> > >             (modify-frame-parameters
> > >              frame (list (cons 'cursor-type 'box)))
> > >             (modify-frame-parameters
> > >              frame (list (cons 'cursor-color "#00A9FE"))))
> > >         (progn
> > >           (modify-frame-parameters
> > >            frame (list (cons 'cursor-type 'hbar)))
> > >           (modify-frame-parameters
> > >            frame (list (cons 'cursor-color "green")))
> > >           )))))
> > >
> > > (defun test/enter-minibuffer()
> > >   (setq box-cursor nil)
> > >   (test/set-cursor))
> > >
> > > (defun test/exit-minibuffer()
> > >   (setq box-cursor t)
> > >   (test/set-cursor))
> > >
> > >
> > > (add-hook 'window-state-change-hook #'test/enter-minibuffer)
> > > (add-hook 'window-state-change-hook #'test/exit-minibuffer)
> > >
> > > (server-start)
> > > (make-frame
>
> > > The only difference is the add-hook, this time using
> > > window-state-change-hook instead of minibuffer-...
> > > This leads to the same bug.
>
> > > Regards,
>
> > > Iris.
>

[-- Attachment #2: Type: text/html, Size: 3465 bytes --]