bug#745: pop-to-buffer, frames, and input focus

[Helmut Eller]

* martin rudalics [2008-08-25 15:45+0200] writes:

[...]
> So when `display-buffer' has called `pop-up-frame-function' we always
> anticipate that the new frame is mapped (visible), has got input focus,
> and is raised.  We cannot remove input focus from it or lower it because
> we would risk some nasty race conditions with the window manager. The
> only reasonable alternative would be to make the new frame Withdrawn and
> let `pop-to-buffer' etc. activate it but this seems hardly manageable.
>
> We should document the current behavior to avoid going through these
> torments again.

At the risk of repeating what you said, the current implementation can't
assume whether the frame created by display-buffer has got focus or not.
It's the window manager who decides that.

We only know definitely that a particular frame has got focus if we
receive a FocusIn event.  But display-buffer (which AFAICT ultimately
calls x_make_frame_visible) only waits until the frame is mapped.

[I found this entertaining comment in xterm.c:x_make_frame_visible:

    /* Wait until the frame is visible.  Process X events until a
       MapNotify event has been seen, or until we think we won't get a
       MapNotify at all..  */

In other words, we don't even know that the frame is mapped!]

However, even documenting that the input focus is unspecified would be
of some use.

>>>> (If the
>>>> buffer is already visible in some frame, that frame is raised but the
>>>> focus is not moved to that frame.  This is IMO correct.)
>>> I think so.  But what happens in the `pop-to-buffer' case when a _new_
>>> frame gets displayed.  Should we give it input-focus?
>>
>> Yes.  I don't see why not.
>
> And raise it I presume.

Yes.

>  IIUC we currently rely on the window-manager to
> raise and focus the frame.

Currently Emacs does both.  Requesting the focus from the X server
(XSetInputFocus) and telling the window manager to activate
(x_ewmh_activate_frame [which for most WMs is focus+raise]) the frame.

>>> I suppose so from
>>> your situation B comments below.  So in all cases where situation A has
>>> Y (that is `display-buffer' fails TDTRT) `pop-to-buffer' DTRT?
>>
>> Yes, right.  In this case pop-up-frames must be t when pop-to-buffer is
>> called.
>
> Is `pop-to-buffer' free of complications when activating a just created
> frame?  From your understanding would this be a NOOP for most window
> managers?  

Good question.  I guess the usual sequence of events is this:

1. Emacs tries to map a frame and waits for notification
2. WM intercepts map request
3. WM maps the frame + gives focus to frame + raises frame
4. Emacs receives MapNotify

If we add a fifth step:

5. Emacs requests focus and informs WM to activate the frame

we probably do something twice, but activating the same frame twice
shouldn't hurt.  We have a problem if the WM decides to activate a
different frame, but that seems unlikely.

> Note that `x-focus-frame' uses XSetInputFocus together with
> x_ewmh_activate_frame.  This mixture seems slightly frightening.

Yes, this looks like "programming by accident".

>>> Note in this context
>>> that `select-frame-set-input-focus' always raises the frame.
>>
>> Looks unnecessary.
>
> For Metacity, maybe, because of their "can't raise without focusing"
> invariant read backwards.  But would it harm?  IIRC some window-managers
> may also move frames between desktops/workspaces when raising them.

The only harm, that I can think of right now, is the understandability
of the implementation :-)

>> Emacs also calls XSetInputFocus together with
>> x_ewmh_activate_frame in Fx_focus_frame.  One of those is probably
>> unnecessary.  x_ewmh_activate_frame tells the WM to try to focus the
>> frame, but some WMs will also raise the window at the same time.
>>
>>>> ?  N  Metacity   follow-mouse (mouse)
>>>> ?  N  Metacity   follow-mouse (sloppy)
>>>> ?  N  Metacity   click-to-focus (click)
>>> Does Metacity respect `select-frame-set-input-focus' at all?
>>
>> Yes, this seems to work.  [Though, it also moves the mouse pointer to
>> the upper right corner of the new frame.  Maybe some Metacity
>> idiosyncrasy.]
>
> Does it do so even with `focus-follows-mouse' nil?

select-frame-set-input-focus seems to work identical for
focus-follows-mouse and click-to-focus mode.  The frame receives the
input focus, is raised, and the mouse pointer is moved to its upper
right corner.

> Anyway: I could imagine something like the (completely untested) snippet
> below to handle situation B - modulo whatever we should change in
> `select-frame-set-input-focus', `x-focus-frame', ...
>
>
> (defcustom pop-up-frame-activate nil
>   "When non-nil try to explicitly activate popped up frames.
> If this is nil leave it to Emacs how to activate the frame.  If
> this is t always try to activate a new frame.  Anything else
> means activate a frame if and only if it existed before the most
> recent call to `display-buffer' ..."
>   :type '(choice (const :tag "System Dependent" nil)
>                (const :tag "Existing frames only" 'existing-only)
>                (const :tag "Always" t))
>   :group 'frames)

I'm not sure that this issue must be customizable at all.  As a user of
pop-to-buffer, I don't quite see the usefulness of the "System
Dependent" and "Existing frames only" choices.  An Emacs implementor
could argue that "System Dependent" is simpler to implement, but if the
more complicated variants are implemented too it's no longer "simpler".

I propose this change instead:

(defun pop-to-buffer (buffer-or-name &optional other-window norecord)
  "Select buffer BUFFER-OR-NAME in some window, preferably a different one.
... "
  (let ((buffer
         ;; FIXME: This behavior is carried over from the previous C version
         ;; of pop-to-buffer, but really we should use just
         ;; `get-buffer' here.
         (if (null buffer-or-name) (other-buffer (current-buffer))
           (or (get-buffer buffer-or-name)
               (let ((buf (get-buffer-create buffer-or-name)))
                 (set-buffer-major-mode buf)
                 buf)))))
    (set-buffer buffer)
    (let ((old-frame (selected-frame)))
      (select-window (display-buffer buffer other-window) norecord)
      (unless (eq old-frame (selected-frame))
        ;; select-window doesn't set the input focus.  Set it explicitly.
        ;; FIXME: select-window should request focus (perhaps lazily).
        (select-frame-set-input-focus (select-frame))))
    buffer))


Helmut.