Subject: bug#33498: 26.1; Unable to delete minibuffer-only+child frames

From: martin rudalics <[email-masked]>
Date: Tue, 27 Nov 2018 11:14:35 +0100

>> These operations would have to be automatized and improved as follows:
 >>
 >> Process a (minibuffer . child-frame) frame parameter to
 >>
 >> (1) make an _invisible_ top-level minibuffer-only frame similar to
 >>      what we are doing in the (minibuffer . nil) case,
 >>
 >> (2) create the minibuffer-less parent frame with the minibuffer window
 >>      set to the window made in (1),
 >>
 >> (3) reparent the minibuffer frame created in (1) and make it visible.
 >
 > Should this be a dedicated function,
 > e.g. make-frame-with-minibuffer-child.  Or a behavior triggered by an
 > additional frame-parameter passed to the parent ?

Ideally, the behavior would be triggered by a special value of the
'minibuffer' frame parameter specified via 'initial-frame-alist' or
'default-frame-alist'.  On window systems where child frames are not
supported, it would automatically fall back on the (minibuffer . nil)
behavior.

(minibuffer . child-frame) would be one way to specify that.  Note
that the value 'child-frame' would not get stored in the child-frame's
minibuffer parameter: the parent frame's minibuffer parameter would
become nil, the child frame's minibuffer parameter would become
'only'.

All this would be done in 'frame-notice-user-settings' in frame.el
independently from how we eventually try to get rid of the minibuffer
frame.

 >> Then deleting the parent frame would
 >>
 >> (4) make the minibuffer frame invisible and top-level,
 >>
 >> (5) delete the parent frame,
 >>
 >> (6) delete the minibuffer frame if possible or make it visible if it
 >>      still serves as minibuffer frame for another frame.
 >>
 >> (4)-(5) would have to handle the cases correctly where delete_frame
 >> (the C function) is called from Elisp (via C-x 5 0, for example) and
 >> from the window manager (by clicking the "x" on the title bar).  The
 >> Elisp call would not shut down Emacs, the window manager call could.
 >>
 > It seems to me that this code should go into Fdelete_frame ?

No.  AFAICT we have to process the

!other_frames (f, false, !NILP (force))

in delete_frame first (note that our minibuffer child frame is not
reported by that call as separate frame).  Only if that call succeeds
(so another top-level frame exists) we'd make the minibuffer window
top-level and invisible before it gets caught by the delete_frame call
in

   /* Softly delete all frames with this frame as their parent frame or
      as their `delete-before' frame parameter value.  */
   FOR_EACH_FRAME (frames, frame1)
     if (FRAME_PARENT_FRAME (XFRAME (frame1)) == f
	/* Process `delete-before' parameter iff FRAME is not a child
	   frame.  This avoids that we enter an infinite chain of mixed
	   dependencies.  */
	|| (nochild
	    && EQ (get_frame_param (XFRAME (frame1), Qdelete_before), frame)))
       delete_frame (frame1, Qnil);

and remember it in a Lisp variable say minibuffer_child_frame.

Then delete_frame would do its normal job by deleting the parent frame
and any actions related to that.  Note that this could delete, by
side-effect, our minibuffer_child_frame.

At the end of delete_frame we'd check if our minibuffer_child_frame is
still a live frame.  If it has no other clients, we'd delete it via a
separate delete_frame call.  Otherwise, we'd make it visible.

There are (at least) two tricky things to observe: This

   /* delete_frame_functions may have deleted any frame, including this
      one.  */
   if (!FRAME_LIVE_P (f))
     return Qnil;

must instead go to the end of delete_frame instead of trying to delete
minibuffer_child_frame.  And we must guarantee that
minibuffer_child_frame becomes visible when an error occurs within
delete_frame before it reaches the end.  This would be done with the
help of a record_unwind_protect form which would be roughly
implemented as follows:

At the top of delete-frame add a

   ptrdiff_t count = SPECPDL_INDEX ();

When assigning minibuffer_child_frame do

   record_unwind_protect (make_frame_visible_if_live, child_frame);

At the end add

   unbind_to (count, Qnil);

make_frame_visible_if_live would then be a trivial function accepting
a Lisp_Object frame argument and would call Fmake_frame_visible for
that frame provided it is live.

 > What if we allow the deletion of a minibuffer frame, if it will not
 > violate the invariant that all frames have a minibuffer, i.e. because
 > the frame in question will be deleted some time later (because this
 > operation is already on the call-stack.).  I don't know if this
 > temporary violation (i.e. between deleting the child-minibuffer and
 > deleting the parent) is a problem.  Anyway, this would enable the user
 > to delete these kinds of frames normally, even if it looks jerky.

Frame deletion is in C because it's extremely hairy and must be done
in some sort of a atomic fashion.  So any temporary violation of the
"each frame must have a minibuffer window" rule is no solution at the
present.

 > And then implement the special minibuffer-as-child-frame behavior and
 > logic via dedicated functions, which display and undsiplay these frames
 > nicely.  (Still, delete-frame would have to be redirected to some proxy
 > handling this case.)

If window manager calls (clicking the "x" on the title bar) are
processed without problems (that is, delete parent and minibuffer
child frame in one go - did you check that?) then we could have
'delete-frame' run a 'before-delete-frame-functions' hook.  In that
hook we could make the minibuffer top-level and invisible while
'after-delete-frame-functions' would take care of deleting the
minibuffer frame or making it visible.  But if anything bad happens,
then 'after-make-frame-functions' would not get executed and the
minibuffer frame stay invisible forever.  Not making the minibuffer
frame invisible, OTOH would show it for a short while somewhere on the
desktop which doesn't look very professional IMO.

martin