bug#11939: 24.1; `save-buffers-kill-emacs' loses minibuffer focus when itcalls `list-processes'

[Drew Adams]

>  > The function assumes that it is called from the 
>  > minibuffer, i.e., that the selected frame is the standalone frame.
>  > Which it is...
> 
> Why does that function _assume_ that?  It should check it.

It does now.  I did not realize that creation of a new frame by MS Windows with
the minibuffer frame selected (& having the focus) would change the focus.  And
I did not realize that such a change in focus would provoke another invocation
of that function (via command `handle-switch-frame', apparently), with the
minibuffer frame unfocused.

> So `1on1-fit-minibuffer-frame' assumes that the selected frame is the
> standalone minibuffer frame and that that frame has focus.  Why don't
> you verify all that in the function's body?

See above - you already asked that.  It _is_ the minibuffer frame for the call
that I expected.  I did not expect the second call provoked by the frame switch
command (via post-command-hook), with the wrong frame focused.

>  > The minibuffer was still active,
> 
> ... you mean `active-minibuffer-window' returned the window of the
> minibuffer-only frame ...

No, I meant only that the minibuffer was still active: accepting typed input.
My next sentence made it clear that the minibuffer-only frame was NOT selected:

>  > but the selected frame was another one (e.g. *Process List*).

Now maybe there is no real notion of a minibuffer being active, and it is more
correct to speak of an minibuffer _window_ being active.

I was describing things from a (possibly naive) user perspective: a
`read-from-minibuffer' was still in progress; the minibuffer was in principle
available for entering text (but its window was not selected).  IOW, if it were
selected it would accept input.

>  > I came up with three alternative fixes that work - I chose 
>  > the second one:
>  >
>  > 1. The first fix is to call `select-frame-set-input-focus' 
>  > at the end of `1on1-fit-minibuffer-frame'.
>  >
>  > But what's not clear to me is why calling 
>  >`select-frame-set-input-focus' at the
>  > end of the first call to `1on1-fit-minibuffer-frame' fixes 
>  > things.  As I said, the frame switch seems to happen between
>  > the two calls, yet selecting the minibuffer frame before the
>  > end of the first call solves the problem.  Maybe
>  > this has something to do with the redisplay code?  No idea.
> 
> You have two calls from `post-command-hook': The first seems due to
> `save-buffers-kill-emacs' preliminary terminating with a `yes-or-no-p'
> question which does not affect frame or focus.

Yes. The focus is in the minibuffer frame.  AFAICT, each time
`read-from-minibuffer' is called the focus (correctly) moves to that frame.  I
have not noticed any case where that did not happen.  Do you think there are
such cases?

> The second should come from `handle-switch-frame' as a consequence
> of emacs being called back by the window manager.

That's what I surmised also.

> IIUC `handle-switch-frame' calls do_switch_frame with TRACK equal 0,
> so focus is not affected by `handle-switch-frame'.

OK.  I'm not familiar with the code, but it's good to know that `h-s-f' (and
presumably also `switch-frame') do not affect focus.

I have not noticed that.  In fact, I thought that switching frames always did
seem to change the focus.  But perhaps it is something else and not just `h-s-f'
that actually causes the focus change (e.g. when you click another frame with
the mouse).  Perhaps it is (always?) the window mgr that changes the focus?

> But focus has been redirected to the new frame by the window 
> manager and emacs should probably adapt to that situation because
> that is the frame that gets the keystrokes.

Sounds like a plan.

> Now if whatever you want to do after `handle-switch-frame' has
> terminated happens in the new frame, there's no problem.  We have a
> problem if we want to make things happen in another frame and for that
> purpose we have to redirect focus to that other frame.
> 
> That's what you apparently do via the `select-frame-set-input-focus'
> call at the end of the first `post-command-hook' execution since
> `handle-switch-frame' won't change focus afterwards.

Yes, but it's still not clear to me just what is going on here.  The window mgr
changes the focus when it creates the new frame.  But
`1on1-fit-minibuffer-frame' is invoked via `post-command-hook', which means that
there is a command that initiates it.  I was thinking that that command must be
`h-s-f', and my testing seemed to confirm that (see fix #3, below), but if it is
not I would like to know what it really is.

If the focus change happens via the window mgr after
`1on1-fit-minibuffer-frame', and if `h-s-f' then provokes the second call to
`1on1-fit-minibuffer-frame', how can resetting the focus to the minibuffer frame
before the end of the first `1on1-fit-minibuffer-frame' solve the problem?  That
resetting would need to take place after the window mgr changed the focus, no?

It seems therefor like the window mgr changes the focus _before_ the end of the
first `1on1-fit-minibuffer-frame'.  But if I add a `message' call at the end, it
shows that the focus is still in the minibuffer frame.  That is what I do not
understand: On the one hand, the focus seems to remain in the minibuffer frame
throughout the first call to `1-f-m-f'.  On the other hand, explicitly setting
the focus to the minibuffer frame at the end of `1-f-m-f' solves the problem.
Can you explain that?

> Looks like a very fragile hack.

Which is why I picked the second fix.  And I still am not clear why the first
fix works - see previous.

>  > 2. The second fix is to have `1on1-fit-minibuffer-frame' 
>  > do nothing unless:
>  > (eq last-event-frame
>  >     (save-selected-window
>  >      (select-window (minibuffer-window)) (selected-frame)))
> 
> Is that (eq last-event-frame (window-frame (minibuffer-window)))?

Duh.  Thanks.

> This means that you don't do anything in this case so apparently some
> side-effect gets suppressed.  Which side-effect?

What the function does: fit the minibuffer frame to its displayed content.  As I
said, the function should be a no-op if the minibuffer is not active or the
minibuffer frame is not in focus.

FYI, this is the full condition under which it will not be a no-op:

;; We could assume the minibuffer frame is `1on1-minibuffer-frame', but we do
not.
(and 1on1-fit-minibuffer-frame-flag
     (active-minibuffer-window)
     ;; Do this because this command is on `post-command-hook',
     ;; and an event such as `handle-switch-frame' might have
     ;; changed the selected frame.
     (eq last-event-frame (window-frame (minibuffer-window)))
     (save-selected-window
       (select-window (minibuffer-window))
       ;; We should be able to use just (one-window-p),
       ;; but an Emacs bug means we need this:
       (one-window-p nil 'selected-frame)))

Perhaps you see some improvement/simplification possible there.  (I don't recall
what Emacs bug was involved wrt the last clause or what release that bug is in,
FWIW.)

And yes, I know that you feel that `one-window-p' is not the right way to check
for one-windowness.  But I need this to work for multiple Emacs versions, and so
far I've found that `one-window-p' DTRT.  Still, I'm open to suggestions if you
see an improvement here.

>  > 3. The third fix is to have `1on1-fit-minibuffer-frame' do 
>  > nothing unless: `this-command' is not eq to `handle-switch-frame.
> 
> Same as before.  What is the side-effect of `1on1-fit-minibuffer-frame'?

Same as before.  This is just a different way to make the function a no-op.
Testing seemed to show that `this-command' is `handle-switch-frame' in the
problematic case.  

But perhaps this is fragile also (perhaps there are additional commands where it
should be a no-op).  Which is why I picked fix #2.

>  > In sum, this is my guess: The creation of the new frame provoked a
>  > `switch-frame' event, which, because of `post-command-hook' caused
>  > `1on1-fit-minibuffer-frame' to be called a second time, 
>  > this time with the new frame selected because the last command
>  > was `handle-frame-switch'.
> 
> I think you should make sure two things: (1) 
> `1on1-fit-minibuffer-frame' should do something iff the frame in
> question is a minibuffer frame.

What do you mean by "a minibuffer frame"?  It just has a non-nil `minibuffer'
parameter?  Or that parameter has a value of `only'?  Or something else?

Is the following test not sufficient to ensure that it is "a minibuffer frame"?

(eq last-event-frame (window-frame (minibuffer-window)))

Let me know if you think I should add an additional test or replace that test
with another.

> (2) `1on1-fit-minibuffer-frame' should avoid having any 
> side-effects wrt window selection or focus unless you explictly want that.

That is the case - it has no side effects wrt window selection or focus.  The
selection window and the focus are at the end what they were at the beginning.

All it does, when it is not a no-op, is (maybe) resize and reposition the
minibuffer frame.

Thx.