bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

Replied in order, as I read.  If you want to cut to the chase, see the end.

>  >> Did `display-buffer' work correctly?
>  >
>  > Not sure what you mean.  I haven't seen a problem until this build.
>  > Hence the report of this being a regression.
> 
> So you didn't check?

Can you please tell me what to check, and how?  It's not clear to me what
you are asking/suggesting; sorry.

>  >> The problem is that the new frame doesn't yet show the buffer you want
>  >> to display when `after-make-frame-functions' is called.
>  >
>  > I see.  So you are saying that the new frame object is passed to the hook
>  > functions, but that new frame has not yet been displayed.  If so, that is
>  > presumably the cause of the regression.
> 
> No.  I am saying that at the time `after-make-frame-functions' is
> called, the new frame does not yet show the buffer you want to display
> in it via plain `display-buffer'.
> 
>  > What's the point of passing the newly created frame object to hook
>  > functions intended to act on it, if that frame has not yet been displayed
>  > so they can do so?
>  >
>  > Perhaps you are allowing for hook functions that do not assume the frame
>  > is displayed.  Is that the point of this change?  Should I change the hook
>  > function here to, say, (lambda (fr) (raise-frame fr) (fit-frame fr))?  Or
>  > perhaps `make-frame-visible' instead of `raise-frame'?
>  >
>  > I just tried those, and they does not work either.  If I want to apply a
>  > function such as `fit-frame' to the new frame, and it is not yet
>  > displayed, what do I need to call in the hook function to display it first?
> 
> You want to apply `fit-frame' to the buffer you eventually want to
> display on the new frame.  Right?

I want to apply it to the frame (not to a buffer) _after_ it has been
displayed.  Previously, creating the frame (`make-frame') also displayed it,
and it did so before hook `after-create-frame-functions' was invoked.

>  > The doc string of `make-frame' suggests, BTW, that it should both (a)
>  > make a frame object and (b) display it, as I have always thought it did
>  > do and should do.  It says: "Return a newly created frame displaying the
>  > current buffer."
> 
> The _current_ buffer.

The question is which buffer should be current for that frame at that point.

I'm not sure the doc string should really have said "current buffer", as in
`current-buffer'.  Seems like what was intended - what makes sense in terms
of the hook, and what I have always thought has been the behavior until now -
is the buffer the frame was created to display (which is harder to say than
"current buffer").  IOW, the buffer that ends up displayed in the frame.

The key part of that doc string, for me, is that it returns a new frame that
is already displayed.  No one ever _sees_ a newly created frame first display
the `current-buffer' and then switch to the actual buffer to be displayed in
the frame.

What you see is only the frame displayed (immediately) with the proper buffer.
And that displayed frame, with the buffer it displays, is what has always
been passed to the `after-make-frame-functions'.

And previously that frame was always displayed before `after-make-frame-*'
was invoked.  So in a hook function you could do this, for example:

 (save-window-excursion
   (select-frame frame)
   (setq specbuf-p
         (and empty-buf-p
              (special-display-p (buffer-name (window-buffer))))))

And the `window-buffer' was the buffer that the frame displayed, which was
already the buffer that the frame was created to display.

> Because the "since forever" behavior is inconsistent.  Consider the
> following forms:
> 
> (defun mess (frame)
>    (message "selected: %s ... frame: %s ... buffer: %s"
>          (selected-frame) frame
>          (window-buffer (frame-root-window frame))))
> 
> (let ((pop-up-frames t))
>    (add-hook 'after-make-frame-functions 'mess)
>    (display-buffer "*Messages*")
>    (remove-hook 'after-make-frame-functions 'mess))
> 
> (let ((pop-up-frames t))
>    (add-hook 'after-make-frame-functions 'mess)
>    (pop-to-buffer "*Messages*")
>    (remove-hook 'after-make-frame-functions 'mess))
> 
> With emacs -Q evaluate the first and and then try the other two with
> some older version and the current trunk.  You should notice that the
> FRAME argument of `mess' is always different from the selected frame.
> But the buffer FRAME displays is different.  With the older versions the
> buffer is *scratch* for plain `display-buffer' and *Messages* for
> `pop-to-buffer'.  With current trunk it is *scratch* for both.
> 
> So I suppose that since forever your `fit-frame' function works on the
> "wrong" buffer when you call plain `display-buffer' and the buffer you
> want to display is not current at that time.

I see.  Yes, I see the behavior you describe, from emacs -Q.

However, I have never noticed a problem in this regard with my code -
dunno why.  For a moment I wondered if this is perhaps because on MS
Windows a new frame gets a certain kind of focus (not sure how to
characterize that behavior).  But I also use my code on GNU/Linux with
Emacs 21.3, and there too I have never see `fit-frame' fit a new frame
to the wrong buffer (the previously current one) etc.  Mystery.

It is true that I also make other calls to `fit-frame', including on
`temp-buffer-show-hook' for one-window frames.  Perhaps one of those
other calls has been compensating for the discrepency in Emacs -Q that
you point out.  Dunno.

Anyway, here is my question, given this discrepancy.  You want to make
the buffer be consistent.  OK, But you have so far chosen the
`current-buffer' as the buffer to use, perhaps from a reading of the
doc string.  Why not instead choose the buffer that will be displayed
in the frame, consistently?

IOW, for your test above, always have the buffer be *Messages*, since
that is the buffer displayed in the frame that is passed to
`after-make-frame-functions'?  What is the use case for passing the
frame, which will display (or is already displaying?) the *Messages*
buffer, have as its root-window buffer some other buffer (*scratch*) that
is not even displayed in that frame and perhaps never will be?

> What I propose is to use the following as substitute for the old
> `display-buffer-pop-up-frame':
> (defun display-buffer-pop-up-frame (buffer alist)...
> 
> With this the buffer printed by `mess' should be *Messages* for both,
> the plain `display-buffer' case and `pop-to-buffer'.

Sounds like you are suggesting the same thing I suggested immediately
above, and you are providing code that implements that.  And yes, a
quick trial indicates that that does seem to work, so far.  I will
load that code at startup and let you know if I notice any problems.
Thx.