bug#30207: 27.0.50; [PATCH] other-window-for-scrolling returns window on daemon frame

[Basil L. Contovounesios]

0001-Do-not-scroll-other-window-in-daemon-frame.patch
Description: Text Data

Consider the following:

1. emacs -Q --daemon

2. emacsclient --create-frame

3. (selected-window)
     => #<window 3 on *scratch*>

4. (other-window-for-scrolling)
     => #<window 1 on *scratch*>

5. (frame-terminal (window-frame (other-window-for-scrolling)))
     => #<terminal 0 on initial_terminal>

This can result in commands scroll-other-window and
scroll-other-window-down scrolling the "phantom" initial frame of the
daemon instead of signalling a "no other window" error.  I imagine this
behaviour is never desirable.

I believe this issue and its potential resolution share some ground with
bug#27210 in that the daemon frame is counterintuitively considered
visible in the following repeat-until condition in
Fother_window_for_scrolling:

  do
    window = Fnext_window (window, Qnil, Qt);
  while (! FRAME_VISIBLE_P (XFRAME (WINDOW_FRAME (XWINDOW (window))))
         && ! EQ (window, selected_window));

Until a more thorough review of the visibility of the initial daemon
frame is reached, I propose the attached patch to additionally ignore
the daemon frame in the aforementioned repeat-until condition.  I assume
the overhead of the additional loop logic and IS_DAEMON invariant is
negligible compared to the rest of the function.

An alternative approach to the attached patch would be to limit "other
window scrolling" to frames in the current terminal, as per the spirit
of bug#5616:

diff --git a/src/window.c b/src/window.c
index 08c3f32dff..328fcb3829 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5725,11 +5725,8 @@ specifies the window.  This takes precedence over
 
       if (EQ (window, selected_window))
        /* That didn't get us anywhere; look for a window on another
-           visible frame.  */
-       do
-         window = Fnext_window (window, Qnil, Qt);
-       while (! FRAME_VISIBLE_P (XFRAME (WINDOW_FRAME (XWINDOW (window))))
-              && ! EQ (window, selected_window));
+           visible frame on the current terminal.  */
+        window = Fnext_window (window, Qnil, Qvisible);
     }
 
   CHECK_LIVE_WINDOW (window);

or at least prioritise the current terminal's frames.  WDYT?

Thanks,

-- 
Basil

In GNU Emacs 27.0.50 (build 25, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll 
bars)
 of 2018-01-22 built on thunk
Repository revision: c42959cc206bcb52baffd45f892da1b767f0f8c1
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
System Description: Debian GNU/Linux testing (buster)

Configured using:
 'configure --config-cache --prefix=/home/blc/.local --with-mailutils
 --with-x-toolkit=lucid --with-modules --with-file-notification=yes
 --with-x 'CFLAGS=-flto -fomit-frame-pointer -march=native -maes -mavx
 -mcrc32 -mf16c -mfpmath=sse -mfsgsbase -mfxsr -minline-all-stringops
 -mmmx -mpclmul -mpopcnt -msahf -msse4.2 -mxsave -mxsaveopt -mvzeroupper
 -O2 -pipe' LDFLAGS=-flto'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 MODULES THREADS LIBSYSTEMD JSON LCMS2