bug#48408: BUGFIX: window-default-font-height: check for nil string

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

In package windows.el, function `window-default-font-height' calls
function `face-font', which can return a nil value, but performs a
`string-equal' operation on the output without checking for that
possibiliy.

I came across this in an unusual case, of having an 'emacs -nw' server
an 'emacsclient -nw', and opening a GUI emacsclient to connect to it.
From that point forward until the fix, every *second* attempt to use the
minibuffer on the 'emacsclient -nw' would fail.

The fix I'm using is:

    (if (and (display-multi-font-p (frame-parameter frame 'display))
             (stringp default-font)  ; <<<---!!!
	     (not (string-equal (frame-parameter frame 'font) default-font)))

But there could be other considerations to performing things in another way.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Thu, 13 May 2021 21:15:19 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> 
> In package windows.el, function `window-default-font-height' calls
> function `face-font', which can return a nil value, but performs a
> `string-equal' operation on the output without checking for that
> possibiliy.
> 
> I came across this in an unusual case, of having an 'emacs -nw' server
> an 'emacsclient -nw', and opening a GUI emacsclient to connect to it.
> From that point forward until the fix, every *second* attempt to use the
> minibuffer on the 'emacsclient -nw' would fail.

This means the problem is either in window-normalize-window or in
frame-parameter or in display-graphic-p, and should be fixed there.
Can you tell what does the below produce on the TTY frame where you
have this problem?

  M-: (frame-parameter nil 'display) RET
  M-: (window-frame (window-normalize-window nil t)) RET

> The fix I'm using is:
> 
>     (if (and (display-multi-font-p (frame-parameter frame 'display))
>              (stringp default-font)  ; <<<---!!!
> 	     (not (string-equal (frame-parameter frame 'font) default-font)))

No, that's not the right fix, IMO.  The code already takes care of not
assuming default-font is a string when on TTY frames, but the
condition doesn't work in this case for some reason.  We need to
understand why and fix that there.

Thanks.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 > In package windows.el, function `window-default-font-height' calls
 > function `face-font', which can return a nil value, but performs a
 > `string-equal' operation on the output without checking for that
 > possibiliy.

Please report this to the author(s) of that package.

 > I came across this in an unusual case, of having an 'emacs -nw' server
 > an 'emacsclient -nw', and opening a GUI emacsclient to connect to it.
 >  From that point forward until the fix, every *second* attempt to use the
 > minibuffer on the 'emacsclient -nw' would fail.
 >
 > The fix I'm using is:
 >
 >      (if (and (display-multi-font-p (frame-parameter frame 'display))
 >               (stringp default-font)  ; <<<---!!!

What is `default-font'?

 > 	     (not (string-equal (frame-parameter frame 'font) default-font)))
 >
 > But there could be other considerations to performing things in another way.

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 >  > In package windows.el, function `window-default-font-height' calls
 >  > function `face-font', which can return a nil value, but performs a
 >  > `string-equal' operation on the output without checking for that
 >  > possibiliy.
 >
 > Please report this to the author(s) of that package.

Sorry.  This function is missing from my current window.el here ...

 >  > I came across this in an unusual case, of having an 'emacs -nw' server
 >  > an 'emacsclient -nw', and opening a GUI emacsclient to connect to it.
 >  >  From that point forward until the fix, every *second* attempt to use the
 >  > minibuffer on the 'emacsclient -nw' would fail.
 >  >
 >  > The fix I'm using is:
 >  >
 >  >      (if (and (display-multi-font-p (frame-parameter frame 'display))
 >  >               (stringp default-font)  ; <<<---!!!
 >
 > What is `default-font'?

... and so I didn't catch this one either.  I'll try to find out what
I'm doing differently now.

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 14 May 2021 09:15:09 +0200
> 
>  >  > In package windows.el, function `window-default-font-height' calls
>  >  > function `face-font', which can return a nil value, but performs a
>  >  > `string-equal' operation on the output without checking for that
>  >  > possibiliy.
>  >
>  > Please report this to the author(s) of that package.
> 
> Sorry.  This function is missing from my current window.el here ...

Really?  It is still there on today's master.  So I wonder what kind
of revolution do you have waiting in the wings...

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 >> Sorry.  This function is missing from my current window.el here ...
 >
 > Really?  It is still there on today's master.  So I wonder what kind
 > of revolution do you have waiting in the wings...

Nothing special.  Just a function that takes its buffer's face remapping
into account.  But it lacks an alias and is apparently just as broken as
the old one, see below.

martin


(defun window-face-font-height (&optional window buffer-or-name face)
   "Return font height in pixels of FACE for BUFFER-OR-NAME in WINDOW.
WINDOW must be a live window and defaults to the selected one.
BUFFER-OR-NAME may be a buffer or the name of an existing buffer
and defaults to WINDOW's buffer.  FACE must specify a valid face
and defaults to the default face.

The return value accounts for any remapping of FACE's font (see
`face-remapping-alist') in BUFFER-OR-NAME.  If no such remapping
has been specified, return the height of the font of FACE for
WINDOW's frame.  If BUFFER-OR-NAME does not specify WINDOW's
buffer, calculate the return value with WINDOW's buffer
temporarily set to the buffer specified by BUFFER-OR-NAME."
   (let* ((window (window-normalize-window window t))
          (frame (window-frame window)))
     (if (display-multi-font-p frame)
         (let* ((buffer (if buffer-or-name
                            (window-normalize-buffer buffer-or-name)
                          (window-buffer window)))
                (face (or face 'default))
                (font (if (eq window (selected-window))
                          (window--buffer-face-font buffer face)
                        (with-selected-window window
                          (window--buffer-face-font buffer face))))
                (frame-font (frame-parameter frame 'font)))
	  ;; Avoid calling `font-info' if the font whose info we want is
	  ;; the same as the corresponding frame's default font since
	  ;; `font-info' is expensive for some fonts, see bug #14838.
           (if (and font (not (string-equal font frame-font)))
               (aref (font-info font frame) 3)
             (frame-char-height frame)))
       (frame-char-height frame))))

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Cc: boruch_baum@gmx.com, 48408@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 14 May 2021 10:15:58 +0200
> 
>  >> Sorry.  This function is missing from my current window.el here ...
>  >
>  > Really?  It is still there on today's master.  So I wonder what kind
>  > of revolution do you have waiting in the wings...
> 
> Nothing special.  Just a function that takes its buffer's face remapping
> into account.  But it lacks an alias and is apparently just as broken as
> the old one, see below.

Yes, the same problem should affect this version as well, since it
still assumes (correctly) that if the frame supports fonts, the
default font's name is a string.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-14 09:39, Eli Zaretskii wrote:
> Can you tell what does the below produce on the TTY frame where you
> have this problem?
>
>   M-: (frame-parameter nil 'display) RET

":0.0"

>   M-: (window-frame (window-normalize-window nil t)) RET

#<frame F139 0x5599a30eaf90>

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-14 09:09, martin rudalics wrote:
> > In package windows.el, function `window-default-font-height' calls
> > function `face-font', which can return a nil value, but performs a
> > `string-equal' operation on the output without checking for that
> > possibiliy.
>
> Please report this to the author(s) of that package.

I made a typo in calling it 'windows.el' instead of 'window.el'. I am
reporting on the emacs core package 'window.el':
;; Maintainer: emacs-devel@gnu.org

> What is `default-font'?

nil

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sat, 15 May 2021 23:06:18 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 48408@debbugs.gnu.org
> 
> >   M-: (frame-parameter nil 'display) RET
> 
> ":0.0"

That's the cause of the problem: it should say nil instead.

Can you step through framep-on-display in that case and tell what does
each of the expressions in 'or' there yield for that frame?  That is,
each one of these:

  (terminal-live-p ":0.0")
  (framep ":0.0")
  (frames-on-display-list ":0.0")

My bet is that the 3rd one produces the bad result.

Thanks.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-16 07:22, Eli Zaretskii wrote:
>   (terminal-live-p ":0.0")

on tty frame: nil
on gui frame: nil

>   (framep ":0.0")

on tty frame: nil
on gui frame: nil

>   (frames-on-display-list ":0.0")

on tty frame: (#<frame F152 0x5599a3b27720>)
on gui frame: (#<frame F152 0x5599a3b27720>)

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sun, 16 May 2021 00:42:15 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 48408@debbugs.gnu.org
> 
> On 2021-05-16 07:22, Eli Zaretskii wrote:
> >   (terminal-live-p ":0.0")
> 
> on tty frame: nil
> on gui frame: nil
> 
> >   (framep ":0.0")
> 
> on tty frame: nil
> on gui frame: nil
> 
> >   (frames-on-display-list ":0.0")
> 
> on tty frame: (#<frame F152 0x5599a3b27720>)
> on gui frame: (#<frame F152 0x5599a3b27720>)

Thanks.  So you have just one frame in that session, which is a TTY
frame?  I thought it was a session with both GUI and TTY frames, is
that not so?

Anyway, the above probably means we set up frame parameters
incorrectly somewhere.  Hmm...

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-16 08:25, Eli Zaretskii wrote:
> > Date: Sun, 16 May 2021 00:42:15 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> > Cc: 48408@debbugs.gnu.org
> >
> > On 2021-05-16 07:22, Eli Zaretskii wrote:
> > >   (terminal-live-p ":0.0")
> >
> > on tty frame: nil
> > on gui frame: nil
> >
> > >   (framep ":0.0")
> >
> > on tty frame: nil
> > on gui frame: nil
> >
> > >   (frames-on-display-list ":0.0")
> >
> > on tty frame: (#<frame F152 0x5599a3b27720>)
> > on gui frame: (#<frame F152 0x5599a3b27720>)
>
> Thanks.  So you have just one frame in that session, which is a TTY
> frame?  I thought it was a session with both GUI and TTY frames, is
> that not so?

I have several frames: at least one is definitely a gui frame (ie. it
appears in its own gui 'window' after being created via a gui menu which
seems to have accessed a gui emacsclient.desktop file), and at least one
is definitely a tty frame (ie. my original 'emacs -nw' invocation, from
within tmux).

In performing the final test again:

> > >   (frames-on-display-list ":0.0")

on tty frame: (#<frame F153 0x5599a3a9d710>)
on gui frame: (#<frame F153 0x5599a3a9d710>)

So the output remains the same, but what you didn't ask me to report was
the mode line output, which differs:

on tty frame: -UUU:@**--F139  *Ibuffer*
on gui frame: -UUU:@**--      *Scratch*

Yes, the mode line displays a frame ID different than the
frames-on-display-list output.

Frame 153 does exist: It is the frame created by my email client (mutt)
to compose this email message). So, it would be the most recent tty
frame created, although the GUI frame was created after.

Changing buffers on either frame does not alter the mode-line output for
frame.

Performing 'C-x 5 b' (M-x select-frame-by-name) on either gui or the
non-mutt tty frame only offers one completion candidate option: F139.
The mutt tty frame offers the single completion candidate F153.

Manually create a second frame on the non-mutt tty (C-x 5 2, C-x 5 ,foo).

Perform M-x select-frame-by-name again, and on the non-mutt tty frame
get two completion options: F139 and foo. On the mutt tty frame, just
F153. On the GUI frame, although the mode line hasn't changed, the only
completion candidate is 'emacs@E15' (E15 being the device hostname).
This completion candidate never appears in the tty frame.

> Anyway, the above probably means we set up frame parameters
> incorrectly somewhere.  Hmm...

Can you reproduce this?

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sun, 16 May 2021 08:25:28 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 48408@debbugs.gnu.org
> 
> > on tty frame: (#<frame F152 0x5599a3b27720>)
> > on gui frame: (#<frame F152 0x5599a3b27720>)
> 
> Thanks.  So you have just one frame in that session, which is a TTY
> frame?  I thought it was a session with both GUI and TTY frames, is
> that not so?
> 
> Anyway, the above probably means we set up frame parameters
> incorrectly somewhere.  Hmm...

Turns out it's a feature, see this comment in server.el:

       ;; Note: TTY frames still get a `display' parameter set to the value of
       ;; $DISPLAY.  This is useful when running from that tty frame
       ;; sub-processes that want to connect to the X server, but that means we
       ;; have to be careful here not to be tricked into thinking those frames
       ;; are on `display'.

Boruch, can you try the patch below?

Martin, any comments on this change?  AFAIU we had this subtlety
waiting to bite us since about forever.

diff --git a/lisp/frame.el b/lisp/frame.el
index aff1d47..f6d6ddd 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2025,11 +2025,23 @@ frame-size-changed-p
 
 (declare-function msdos-mouse-p "dosfns.c")
 
+(defun frame-type (&optional display)
+  "Return the type of frames on DISPLAY.
+DISPLAY can be a display name, a frame, or nil (meaning the selected
+frame's display).
+This function is almost like `framep-on-display', but it is safer
+when a session has both text-mode and GUI client frames, because
+text-mode frames can still have a non-nil `display' frame parameter."
+  (cond
+   ((null display) (framep (selected-frame)))
+   ((framep display) (framep display))
+   (t (framep-on-display display))))
+
 (defun display-mouse-p (&optional display)
   "Return non-nil if DISPLAY has a mouse available.
 DISPLAY can be a display name, a frame, or nil (meaning the selected
 frame's display)."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((eq frame-type 'pc)
       (msdos-mouse-p))
@@ -2062,7 +2074,8 @@ display-graphic-p
 that use a window system such as X, and false for text-only terminals.
 DISPLAY can be a display name, a frame, or nil (meaning the selected
 frame's display)."
-  (not (null (memq (framep-on-display display) '(x w32 ns)))))
+  (let ((frame-type (frame-type display)))
+    (not (null (memq frame-type '(x w32 ns))))))
 
 (defun display-images-p (&optional display)
   "Return non-nil if DISPLAY can display images.
@@ -2083,7 +2096,7 @@ display-selections-p
 via special system buffers called `selection' or `clipboard'.
 DISPLAY can be a display name, a frame, or nil (meaning the selected
 frame's display)."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((eq frame-type 'pc)
       ;; MS-DOS frames support selections when Emacs runs inside
@@ -2099,7 +2112,7 @@ display-symbol-keys-p
   "Return non-nil if DISPLAY supports symbol names as keys.
 This means that, for example, DISPLAY can differentiate between
 the keybinding RET and [return]."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (or (memq frame-type '(x w32 ns pc))
         ;; MS-DOS and MS-Windows terminals have built-in support for
         ;; function (symbol) keys
@@ -2111,7 +2124,7 @@ display-screens
   "Return the number of screens associated with DISPLAY.
 DISPLAY should be either a frame or a display name (a string).
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-screens display))
@@ -2131,7 +2144,7 @@ display-pixel-height
 refers to the pixel height for all physical monitors associated
 with DISPLAY.  To get information for each physical monitor, use
 `display-monitor-attributes-list'."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-pixel-height display))
@@ -2151,7 +2164,7 @@ display-pixel-width
 refers to the pixel width for all physical monitors associated
 with DISPLAY.  To get information for each physical monitor, use
 `display-monitor-attributes-list'."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-pixel-width display))
@@ -2191,7 +2204,7 @@ display-mm-height
 refers to the height in millimeters for all physical monitors
 associated with DISPLAY.  To get information for each physical
 monitor, use `display-monitor-attributes-list'."
-  (and (memq (framep-on-display display) '(x w32 ns))
+  (and (memq (frame-type display) '(x w32 ns))
        (or (cddr (assoc (or display (frame-parameter nil 'display))
 			display-mm-dimensions-alist))
 	   (cddr (assoc t display-mm-dimensions-alist))
@@ -2212,7 +2225,7 @@ display-mm-width
 refers to the width in millimeters for all physical monitors
 associated with DISPLAY.  To get information for each physical
 monitor, use `display-monitor-attributes-list'."
-  (and (memq (framep-on-display display) '(x w32 ns))
+  (and (memq (frame-type display) '(x w32 ns))
        (or (cadr (assoc (or display (frame-parameter nil 'display))
 			display-mm-dimensions-alist))
 	   (cadr (assoc t display-mm-dimensions-alist))
@@ -2228,7 +2241,7 @@ display-backing-store
 the question is inapplicable to a certain kind of display.
 DISPLAY can be a display name or a frame.
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-backing-store display))
@@ -2241,7 +2254,7 @@ display-save-under
   "Return non-nil if DISPLAY's screen supports the SaveUnder feature.
 DISPLAY can be a display name or a frame.
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-save-under display))
@@ -2254,7 +2267,7 @@ display-planes
   "Return the number of planes supported by DISPLAY.
 DISPLAY can be a display name or a frame.
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-planes display))
@@ -2269,7 +2282,7 @@ display-color-cells
   "Return the number of color cells supported by DISPLAY.
 DISPLAY can be a display name or a frame.
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-color-cells display))
@@ -2286,7 +2299,7 @@ display-visual-class
 `static-color', `pseudo-color', `true-color', or `direct-color'.
 DISPLAY can be a display name or a frame.
 If DISPLAY is omitted or nil, it defaults to the selected frame's display."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((memq frame-type '(x w32 ns))
       (x-display-visual-class display))
@@ -2342,7 +2355,7 @@ display-monitor-attributes-list
 in a graphical display is dominated by exactly one physical
 monitor at a time, though it can span multiple (or no) physical
 monitors."
-  (let ((frame-type (framep-on-display display)))
+  (let ((frame-type (frame-type display)))
     (cond
      ((eq frame-type 'x)
       (x-display-monitor-attributes-list display))

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sun, 16 May 2021 02:05:12 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: 48408@debbugs.gnu.org
> 
> I have several frames: at least one is definitely a gui frame (ie. it
> appears in its own gui 'window' after being created via a gui menu which
> seems to have accessed a gui emacsclient.desktop file), and at least one
> is definitely a tty frame (ie. my original 'emacs -nw' invocation, from
> within tmux).
> 
> In performing the final test again:
> 
> > > >   (frames-on-display-list ":0.0")
> 
> on tty frame: (#<frame F153 0x5599a3a9d710>)
> on gui frame: (#<frame F153 0x5599a3a9d710>)
> 
> So the output remains the same, but what you didn't ask me to report was
> the mode line output, which differs:
> 
> on tty frame: -UUU:@**--F139  *Ibuffer*
> on gui frame: -UUU:@**--      *Scratch*
> 
> Yes, the mode line displays a frame ID different than the
> frames-on-display-list output.
> 
> Frame 153 does exist: It is the frame created by my email client (mutt)
> to compose this email message). So, it would be the most recent tty
> frame created, although the GUI frame was created after.

Quite a mess, huh.

> Can you reproduce this?

No, I don't have access to a system where I can create this situation,
sorry.

But I sent a potential fix a few moments ago.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sun, 16 May 2021 02:59:05 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> 
> On 2021-05-16 09:29, Eli Zaretskii wrote:
> > Boruch, can you try the patch below?
> 
> The hunks don't patch cleanly, but I can try doing it
> manually ... after yom tov. Chag Sameyach.

Which is a Good Thing, because I've changed my mind.  Please try the
much simpler patch below.

Martin, same question about this one.  I decided that there's nothing
wrong with how we compute frame-type in the display-*-p predicates;
it's window-default-font-height that trusts the 'display' frame
parameter too much.  WDYT?

diff --git a/lisp/window.el b/lisp/window.el
index 026cde5..4b99b77 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8786,7 +8786,11 @@ window-default-font-height
   (let* ((window (window-normalize-window window t))
 	 (frame (window-frame window))
 	 (default-font (face-font 'default frame)))
-    (if (and (display-multi-font-p (frame-parameter frame 'display))
+    ;; Client frames can have the 'display' parameter set like for X
+    ;; frames, even though they are TTY frames, so make sure we won't
+    ;; be duped by that up front with 'framep'.
+    (if (and (not (eq (framep frame) t))
+             (display-multi-font-p (frame-parameter frame 'display))
 	     (not (string-equal (frame-parameter frame 'font) default-font)))
         (aref (font-info default-font frame) 3)
       (frame-char-height frame))))

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 > Martin, any comments on this change?  AFAIU we had this subtlety
 > waiting to bite us since about forever.

I've been always confused by the polymorphism of arguments like TERMINAL
or DISPLAY so I'm hardly qualified to comment on this.  Just install it.

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 > Martin, same question about this one.  I decided that there's nothing
 > wrong with how we compute frame-type in the display-*-p predicates;
 > it's window-default-font-height that trusts the 'display' frame
 > parameter too much.  WDYT?

OK.  But was Boruch's original idea just checking for

(stringp default-font)

worse?

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Cc: 48408@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sun, 16 May 2021 10:35:31 +0200
> 
>  > Martin, same question about this one.  I decided that there's nothing
>  > wrong with how we compute frame-type in the display-*-p predicates;
>  > it's window-default-font-height that trusts the 'display' frame
>  > parameter too much.  WDYT?
> 
> OK.  But was Boruch's original idea just checking for
> 
> (stringp default-font)
> 
> worse?

I'm asking for your opinion about that ;-)

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Sun, 16 May 2021 11:56:45 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: boruch_baum@gmx.com, 48408@debbugs.gnu.org
> 
> > OK.  But was Boruch's original idea just checking for
> > 
> > (stringp default-font)
> > 
> > worse?
> 
> I'm asking for your opinion about that ;-)

Actually, no, I do have an opinion on that: face-font can return a
string even on a TTY frame, at least in a build --without-x.  (Which
seems to be an inconsistency in and of itself, btw.)

In general, trusting frame parameters too much is IME not a good idea,
as they are too loosely defined.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 > Actually, no, I do have an opinion on that: face-font can return a
 > string even on a TTY frame, at least in a build --without-x.  (Which
 > seems to be an inconsistency in and of itself, btw.)
 >
 > In general, trusting frame parameters too much is IME not a good idea,
 > as they are too loosely defined.

Is it possible that `window-font-width' and `window-font-height' run
into a similar problem when `face-font' returns nil?

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Cc: boruch_baum@gmx.com, 48408@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sun, 16 May 2021 11:28:00 +0200
> 
>  > Actually, no, I do have an opinion on that: face-font can return a
>  > string even on a TTY frame, at least in a build --without-x.  (Which
>  > seems to be an inconsistency in and of itself, btw.)
>  >
>  > In general, trusting frame parameters too much is IME not a good idea,
>  > as they are too loosely defined.
> 
> Is it possible that `window-font-width' and `window-font-height' run
> into a similar problem when `face-font' returns nil?

I don't think so, because they call display-multi-font-p without
arguments.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-16 11:30, Eli Zaretskii wrote:
> Which is a Good Thing, because I've changed my mind.  Please try the
> much simpler patch below.
> ...
> -    (if (and (display-multi-font-p (frame-parameter frame 'display))
> +    (if (and (not (eq (framep frame) t))
> +             (display-multi-font-p (frame-parameter frame 'display))

This patch does do the job; The bug does not happen with this patch.

Two other things, though:

1) In working on this bug, I noticed that the mode line on the GUI frame
   was not correctly indicating the frame name. Should I file that as a
   separate bug? Add it to some other current outstanding bug? Let it
   stand as part of this report?

2) Likewise, for more than two years I've intermittently been having a
   'grave' emacs bug that I never reported on this list because I
   couldn't ever figure out how to reproduce it. In working on this bug,
   I seem to have figured out the problem (but not a solution):

   2.1) Whenever a minibuffer is active in one frame, the other windows
        on that frame are navigable and operable. However, all elements
        of all other frames are completely frozen.

   2.2) This most commonly seems to have been happening to me when
        composing an email message using mutt, which I have configured
        to use emacsclient as its editor.

   2.3) Up until now, my work-around has been to `pkill emacsclient` and
        restart the client (this doesn't cause any data loss, since all
        data is on the server).


--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[martin rudalics]

 > 1) In working on this bug, I noticed that the mode line on the GUI frame
 >     was not correctly indicating the frame name.

What do you use to indicate the frame name in the mode line?  If you
have a title bar, does it correctly display the frame name?  Note also
that "the" mode line is ambiguous - with split windows a frame usually
has several mode lines.

 >     2.1) Whenever a minibuffer is active in one frame, the other windows
 >          on that frame are navigable and operable. However, all elements
 >          of all other frames are completely frozen.

What happens when you type C-x 5 o or C-x 5 2 in the navigable frame?

martin

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-19 09:45, martin rudalics wrote:
> > 1) In working on this bug, I noticed that the mode line on the GUI frame
> >     was not correctly indicating the frame name.
>
> What do you use to indicate the frame name in the mode line?

Nothing. It's however the default mode-line is configured. From the
describe-variable page for `mode-line-format':

("%e" mode-line-front-space mode-line-mule-info mode-line-client
mode-line-modified mode-line-remote mode-line-frame-identification
mode-line-buffer-identification " " mode-line-position
 (vc-mode vc-mode) " " mode-line-modes mode-line-misc-info
mode-line-end-spaces)

> If you have a title bar, does it correctly display the frame name?

Yes.

> Note also that "the" mode line is ambiguous - with split windows a
> frame usually has several mode lines.

Performing 'C-x 3' does present two mode lines, both of which fail to
identify the GUI frame. In both cases, the mode line(s) read(s):

   U:@**-   *scratch*   All of 0     (1,0)

>
> >     2.1) Whenever a minibuffer is active in one frame, the other windows
> >          on that frame are navigable and operable. However, all elements
> >          of all other frames are completely frozen.
>
> What happens when you type C-x 5 o or C-x 5 2 in the navigable frame?

Initially, I thought the system responded fine to either commands, but
then got a shock. I used a GUI frame for testing the navigable frame,
Pressed M-x, then C-x 5 2. A second GUI frame *was* created. However,
upon performing Alt-TAB to have the underlying operating system (window
manager, really) switch me to the tmux window in which I had prior emacs
-nw frames, all were frozen and remained in their frozen state until
exiting the minibuffer (The M-x on the GUI frame). What's more
surprising to me was that when I tried M-x C-x 5 0 (yes, I misread your
suggestion of 'o' as zero), the -nw frames also froze, even though the
GUI frame no longer existed! I wasn't expecting that at all. To top
things off, the -nw frames unfroze when re-launching a GUI frame (using
the operating system's xdg-desktop method). When the -nw frames
un-froze, they displayed the 'type-ahead' characters that I mashed while
the frames were frozen.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Wed, 19 May 2021 00:00:53 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: martin rudalics <rudalics@gmx.at>, 48408@debbugs.gnu.org
> 
> 1) In working on this bug, I noticed that the mode line on the GUI frame
>    was not correctly indicating the frame name. Should I file that as a
>    separate bug? Add it to some other current outstanding bug? Let it
>    stand as part of this report?

GUI frame or TTY frame?  GUI frames by default don't have names, you
need to give them a name if you want.

If all I said above doesn't help, then yes, please report a bug with
all the details.

> 2) Likewise, for more than two years I've intermittently been having a
>    'grave' emacs bug that I never reported on this list because I
>    couldn't ever figure out how to reproduce it. In working on this bug,
>    I seem to have figured out the problem (but not a solution):
> 
>    2.1) Whenever a minibuffer is active in one frame, the other windows
>         on that frame are navigable and operable. However, all elements
>         of all other frames are completely frozen.
> 
>    2.2) This most commonly seems to have been happening to me when
>         composing an email message using mutt, which I have configured
>         to use emacsclient as its editor.
> 
>    2.3) Up until now, my work-around has been to `pkill emacsclient` and
>         restart the client (this doesn't cause any data loss, since all
>         data is on the server).

It's a "feature": Emacs can read input only from one frame at a time.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Boruch Baum]

On 2021-05-19 14:29, Eli Zaretskii wrote:
> > Date: Wed, 19 May 2021 00:00:53 -0400
> > From: Boruch Baum <boruch_baum@gmx.com>
> > Cc: martin rudalics <rudalics@gmx.at>, 48408@debbugs.gnu.org
>
> > 2) Likewise, for more than two years I've intermittently been having a
> >    'grave' emacs bug that I never reported on this list because I
> >    couldn't ever figure out how to reproduce it. In working on this bug,
> >    I seem to have figured out the problem (but not a solution):
> >
> >    2.1) Whenever a minibuffer is active in one frame, the other windows
> >         on that frame are navigable and operable. However, all elements
> >         of all other frames are completely frozen.
> >
> >    2.2) This most commonly seems to have been happening to me when
> >         composing an email message using mutt, which I have configured
> >         to use emacsclient as its editor.
> >
> >    2.3) Up until now, my work-around has been to `pkill emacsclient` and
> >         restart the client (this doesn't cause any data loss, since all
> >         data is on the server).
>
> It's a "feature": Emacs can read input only from one frame at a time.

Without that feature, the emacs 'server' functionality could be  useful
for >1 physical 'seats' (in X11/Xorg terms) or >1 separate IP addresses
by >1 individuals. There had been attempts in the past at some FOSS
version of 'collaborative editing' (eg. gobby), and I thought emacs had
that covered with its server/client option.

I'm looking at the emacswiki now ... and as so often the case it seems
very out-of-date.

I don't personally have any immediate or foreseeable need for
collaborative editing, but is there a 'recommended' method?

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Wed, 19 May 2021 07:55:37 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: rudalics@gmx.at, 48408@debbugs.gnu.org
> 
> > It's a "feature": Emacs can read input only from one frame at a time.
> 
> Without that feature, the emacs 'server' functionality could be  useful
> for >1 physical 'seats' (in X11/Xorg terms) or >1 separate IP addresses
> by >1 individuals. There had been attempts in the past at some FOSS
> version of 'collaborative editing' (eg. gobby), and I thought emacs had
> that covered with its server/client option.

Collaborative editing is much more than just getting input from
several sources.

> I'm looking at the emacswiki now ... and as so often the case it seems
> very out-of-date.
> 
> I don't personally have any immediate or foreseeable need for
> collaborative editing, but is there a 'recommended' method?

Someone posted here about a year ago describing the effort to add
collaborative editing to Emacs, look it up in the archives.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Eli Zaretskii]

> Date: Wed, 19 May 2021 00:00:53 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: martin rudalics <rudalics@gmx.at>, 48408@debbugs.gnu.org
> 
> On 2021-05-16 11:30, Eli Zaretskii wrote:
> > Which is a Good Thing, because I've changed my mind.  Please try the
> > much simpler patch below.
> > ...
> > -    (if (and (display-multi-font-p (frame-parameter frame 'display))
> > +    (if (and (not (eq (framep frame) t))
> > +             (display-multi-font-p (frame-parameter frame 'display))
> 
> This patch does do the job; The bug does not happen with this patch.

Thanks, installed.

bug#48408: BUGFIX: window-default-font-height: check for nil string

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> This patch does do the job; The bug does not happen with this patch.
>
> Thanks, installed.

Skimming this bug report, it seems like the reported issue here was
fixed, so I'm closing it.  There were alternative patches discussed, but
they were then discarded, I think?  If there's more to be done here,
please respond to the debbugs address and we'll reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no