bug#73768: 'read-passwd-mode' can clobber user defined mode lines

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

After reading a password here I sometimes end up with seeing one or two
eye icons in mode lines of buffers.  Other buffers may get a blank mode
line.  I suppose the culprit is

     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
       (when (listp mode-line-format)
         (setq mode-line-format (cdr mode-line-format)))))

which can do something arbitrary to user defined mode lines which I use.
Eventually, I have to kill or revert all involved buffers every time
I've read a password.

At the very least the code should try to identify the item it added
earlier, remove the car only if it matches that item and restore the
previous mode line format in case the items don't match.  Also it should
check whether 'read-passwd--mode-line-buffer' still exists.

In either case, it would be nice to make showing the eye icon
customizable by the user.  I found no way to do that.

Thanks, martin

In GNU Emacs 31.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version
  3.24.38, cairo version 1.16.0) of 2024-10-12 built on restno
Repository revision: 6e80509099e27e237787186f4e8731bfdfb9b00d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
  'configure --with-gif=ifavailable --with-tiff=ifavailable
  --with-gnutls=no --without-pop 'CFLAGS=-O3 -g3 -no-pie''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GSETTINGS HARFBUZZ JPEG LIBSELINUX MODULES
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: de_AT.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: ELisp/d

Minor modes in effect:
   pop-up-mini-mode: t
   scroll-restore-mode: t
   regexp-lock-mode: t
   eldoc-tooltip-mode: t
   tooltip-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   context-menu-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   minibuffer-regexp-mode: t
   column-number-mode: t
   line-number-mode: t
   auto-fill-function: my-lisp-mode-auto-fill
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   temp-buffer-resize-mode: t

Load-path shadows:
None found.

Features:
(shadow mail-extr warnings emacsbug message mailcap yank-media puny
rfc822 mml mml-sec password-cache epa epg rfc6068 epg-config gnus-util
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs vc-git files-x
vc-dispatcher elp vc-hg diff-mode track-changes ediff-vers ediff
ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util local-tags goto-last-change speck pop-up-mini info-look
find-func srt-mode derived elinfo-support elinfo texinfo
texinfo-loaddefs info shell pcomplete comint subr-x ansi-osc ansi-color
sidebar bookmark text-property-search sort m&d scroll-restore
regexp-lock time-stamp cl-macs eldoc-tooltip pcase easy-mmode etags
fileloop generator xref project byte-opt bytecomp byte-compile ring
find-dired dired dired-loaddefs cus-edit pp cus-load icons wid-edit
cl-loaddefs cl-lib jka-compr ls-lisp gv rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd touch-screen
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
gtk x-toolkit xinput2 x multi-tty move-toolbar make-network-process
emacs)

Memory information:
((conses 16 271421 29959) (symbols 48 16095 0) (strings 32 44610 3502)
  (string-bytes 1 1330220) (vectors 16 24496)
  (vector-slots 8 277397 25280) (floats 8 109 1037)
  (intervals 56 9458 42) (buffers 984 34))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Martin,

> After reading a password here I sometimes end up with seeing one or two
> eye icons in mode lines of buffers.  Other buffers may get a blank mode
> line.  I suppose the culprit is
>
>     (with-current-buffer read-passwd--mode-line-buffer
>       ;; Remove `read-passwd--mode-line-icon'.
>       (when (listp mode-line-format)
>         (setq mode-line-format (cdr mode-line-format)))))
>
> which can do something arbitrary to user defined mode lines which I use.
> Eventually, I have to kill or revert all involved buffers every time
> I've read a password.
>
> At the very least the code should try to identify the item it added
> earlier, remove the car only if it matches that item and restore the
> previous mode line format in case the items don't match.  Also it should
> check whether 'read-passwd--mode-line-buffer' still exists.

Could you please provide a short recipe I could play with?

> In either case, it would be nice to make showing the eye icon
> customizable by the user.  I found no way to do that.

I'll see what's possible. FTR, non-graphical alternatives are already
defined in read-passwd--show-password-icon and
read-passwd--hide-password-icon.

Could you pls give an example what you would like as replacement?

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Michael

 > Could you please provide a short recipe I could play with?

No, otherwise I would have attached it to my initial post.  It happens
when I run ssh-add in a shell buffer.  But it's not 100% reproducible
and for most of my buffers (car mode-line-format) is invariably "%e".
But I use a separate minibuffer frame so

(window-in-direction 'above (minibuffer-window))

should reliably return nil and (minibuffer-selected-window) would be
used.  'eldoc-mode' is nil so I doubt that it could interfere with its
own ideas about modifying the mode line when I'm in the minibuffer and
possibly prepend the second eye icon I occasionally see.

I'll try to play around with this myself so I can tell you more on what
goes on.

 >> In either case, it would be nice to make showing the eye icon
 >> customizable by the user.  I found no way to do that.
 >
 > I'll see what's possible. FTR, non-graphical alternatives are already
 > defined in read-passwd--show-password-icon and
 > read-passwd--hide-password-icon.
 >
 > Could you pls give an example what you would like as replacement?

Why can't you make that a full-fledged option 'read-passwd-show-icon'
and initialize it to 'graphics-only'?  And why can't you make the icon
part of the prompt with some clickable text or overlay property?  Think
of users who turn mode lines off in some buffers at least.

Thanks, martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

> Hi Michael

Hi Martin,

>>> In either case, it would be nice to make showing the eye icon
>>> customizable by the user.  I found no way to do that.
>>
>> I'll see what's possible. FTR, non-graphical alternatives are already
>> defined in read-passwd--show-password-icon and
>> read-passwd--hide-password-icon.
>>
>> Could you pls give an example what you would like as replacement?
>
> Why can't you make that a full-fledged option 'read-passwd-show-icon'
> and initialize it to 'graphics-only'?

Because Emacs runs also in non-graphical environments.

> And why can't you make the icon
> part of the prompt with some clickable text or overlay property?  Think
> of users who turn mode lines off in some buffers at least.

Minibuffer indications belong to the modeline. And if there is no
modeline, you can always use TAB in the minibuffer to toggle between
password visibility. This is documented in the Elisp manual, (info
"(elisp) Reading a Password") .

Granted, it shall also be mentioned in the Emacs manual, (info "(emacs)
Passwords") . Will do.

> Thanks, martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]

When in my Emacs with a stand alone minibuffer frame I create a shell
buffer and there do ssh-add, the following happens:

In the lines below 'wid' stands for the window in direction (nil in the
rows below because a stand alone minibuffer frame has no window above),
'msw' the minibuffer selected window, 'glw' the largest window (nil in
the first row below because a stand alone minibuffer frame has no
largest window) and 'sw' the selected window.  'add' and 'remove' stand
for adding and removing the icon. 'before' and 'after' stand for whether
we are before or after doing that.  These are followed by the value of
'read-passwd--mode-line-buffer' and the first two elements of that
buffer's mode line string.

wid .. nil .. msw .. #<window 11 on *shell*> .. glw .. nil .. sw .. #<window 8 on  *Minibuf-1*>
add before .. *shell* .. %e .. mode-line-version
add after .. *shell* .. (:eval read-passwd--mode-line-icon) .. %e
wid .. nil .. msw .. nil .. glw .. #<window 6 on .emacs> .. sw .. #<window 11 on *shell*>
remove before .. .emacs .. %e .. mode-line-version
remove after .. .emacs .. mode-line-version ..

So the window chosen for adding the icon is the value of calling
'minibuffer-selected-window' and everything is OK ('mode-line-version'
is a private variable of mine, ignore it).  The icon's buffer is *shell*
and the icon appears correctly.

But when the icon shall be removed, 'minibuffer-selected-window' returns
nil and the largest window is chosen instead which now happens to show
my init file .emacs.  ‘read-passwd’ leaves the icon in *shell* and kills
the mode line of my .emacs file.

The crucial fact is that the selected window changes from the minibuffer
window to the *shell* window because 'minibuffer-selected-window' is
specified as

   if (minibuf_level > 0
       && MINI_WINDOW_P (XWINDOW (selected_window))
       && WINDOW_LIVE_P (minibuf_selected_window))
     return minibuf_selected_window;

and *shell* is not a mini window.  So 'minibuffer-selected-window'
returns nil in the scenario at hand and the largest window of the
selected window's frame is chosen.  And the selected window obviously
changes because 'read-string' (and in further consequence read_minibuf)
restores the window selected before the minibuffer window was selected.

Concludingly, the current version works when 'window-in-direction'
returns a suitable window and may fail miserably otherwise.

The attached patch fixes the problem here.  But I still think that the
icon should appear in the prompt of 'read-passwd' itself.

martin

[-- Attachment #2: read-passwd.diff --]
[-- Type: text/x-patch, Size: 1430 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..5353a7a55fe 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2534,13 +2534,15 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"
 
-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  ;; Stolen from `eldoc-minibuffer-message'.
+  (unless (and (buffer-live-p read-passwd--mode-line-buffer)
+	       (get-buffer-window read-passwd--mode-line-buffer))
+    (setq read-passwd--mode-line-buffer
+          (window-buffer
+           (or (window-in-direction 'above (minibuffer-window))
+	       (minibuffer-selected-window)
+	       (get-largest-window)))))
 
   (if read-passwd-mode
       (with-current-buffer read-passwd--mode-line-buffer
@@ -2552,7 +2554,8 @@ read-passwd-mode
     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
       (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+        (setq mode-line-format (cdr mode-line-format)))
+      (setq read-passwd--mode-line-buffer nil)))
 
   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

Hi Martin,

> When in my Emacs with a stand alone minibuffer frame I create a shell
> buffer and there do ssh-add, the following happens:
>
> In the lines below 'wid' stands for the window in direction (nil in the
> rows below because a stand alone minibuffer frame has no window above),
> 'msw' the minibuffer selected window, 'glw' the largest window (nil in
> the first row below because a stand alone minibuffer frame has no
> largest window) and 'sw' the selected window.  'add' and 'remove' stand
> for adding and removing the icon. 'before' and 'after' stand for whether
> we are before or after doing that.  These are followed by the value of
> 'read-passwd--mode-line-buffer' and the first two elements of that
> buffer's mode line string.
>
> wid .. nil .. msw .. #<window 11 on *shell*> .. glw .. nil .. sw .. #<window 8 on  *Minibuf-1*>
> add before .. *shell* .. %e .. mode-line-version
> add after .. *shell* .. (:eval read-passwd--mode-line-icon) .. %e
> wid .. nil .. msw .. nil .. glw .. #<window 6 on .emacs> .. sw .. #<window 11 on *shell*>
> remove before .. .emacs .. %e .. mode-line-version
> remove after .. .emacs .. mode-line-version ..
>
> So the window chosen for adding the icon is the value of calling
> 'minibuffer-selected-window' and everything is OK ('mode-line-version'
> is a private variable of mine, ignore it).  The icon's buffer is *shell*
> and the icon appears correctly.
>
> But when the icon shall be removed, 'minibuffer-selected-window' returns
> nil and the largest window is chosen instead which now happens to show
> my init file .emacs.  ‘read-passwd’ leaves the icon in *shell* and kills
> the mode line of my .emacs file.
>
> The crucial fact is that the selected window changes from the minibuffer
> window to the *shell* window because 'minibuffer-selected-window' is
> specified as
>
>   if (minibuf_level > 0
>       && MINI_WINDOW_P (XWINDOW (selected_window))
>       && WINDOW_LIVE_P (minibuf_selected_window))
>     return minibuf_selected_window;
>
> and *shell* is not a mini window.  So 'minibuffer-selected-window'
> returns nil in the scenario at hand and the largest window of the
> selected window's frame is chosen.  And the selected window obviously
> changes because 'read-string' (and in further consequence read_minibuf)
> restores the window selected before the minibuffer window was selected.
>
> Concludingly, the current version works when 'window-in-direction'
> returns a suitable window and may fail miserably otherwise.

I tried to reproduce your recipe, but I couldn't. Likely, I have a too
simple minded use of different Emacs windows. And a not customized mode
line. Note, that I'm not familiar with the windows machinery in Emacs at
all.

> The attached patch fixes the problem here.  But I still think that the
> icon should appear in the prompt of 'read-passwd' itself.

Please install your patch (you might ask Eli whether it is good for the
emacs-30 branch). And as I said in the other message, the mode line is
the place to give information about the active buffer, including the minibuffer.

Thanks a lot for investigating it!

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Michael

 > I tried to reproduce your recipe, but I couldn't. Likely, I have a too
 > simple minded use of different Emacs windows. And a not customized mode
 > line. Note, that I'm not familiar with the windows machinery in Emacs at
 > all.

No customized mode line is needed.  Either with emacs -Q evaluate

(progn
   (set-window-parameter nil 'no-other-window t)
   (with-selected-window (split-window nil -5)
     (set-window-buffer nil (get-buffer-create "*text*"))
     (set-window-parameter nil 'no-other-window t)
     (read-passwd "PWD:")))

or with

emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))

evaluate via M-:

(with-selected-window (get-buffer-window "*scratch*" t)
   (with-selected-window (split-window nil -5)
     (set-window-buffer nil (get-buffer-create "*text*"))
     (read-passwd "PWD:")))

In either case it's important that (1) there is no suitable window
"above" and (2) 'minibuffer-selected-window' doesn't return the largest
window on its frame.

martin

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

> Hi Michael

Hi Martin,

> No customized mode line is needed.  Either with emacs -Q evaluate
>
> (progn
>   (set-window-parameter nil 'no-other-window t)
>   (with-selected-window (split-window nil -5)
>     (set-window-buffer nil (get-buffer-create "*text*"))
>     (set-window-parameter nil 'no-other-window t)
>     (read-passwd "PWD:")))

Thanks! With this recipe, I can reproduce the problem. And your patch
fixes it :-)

Please install in the emacs-30 branch.

> martin

Best regards, Michael.

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

 > Thanks! With this recipe, I can reproduce the problem. And your patch
 > fixes it :-)
 >
 > Please install in the emacs-30 branch.

Hold your horses.  First try another recipe: With emacs -Q do C-x 5 2
followed by C-x b to show *Messages* in either of the frames, read a
password, do C-x 5 o and type C-g.  One of the two buffers has the eye
icon the other has the botched mode line.  I'm using the more accurate
patch attached now to hopefully fix that too.  Note that whenever one
switches frames, the mode line showing the eye will always stay with the
initially chosen buffer and not the one where the minibuffer window is.

But there are other scenarios that don't seem right.  While reading one
password ask for reading another one.  You now should see two eye icons
which, however, cannot be attributed to any of the two read operations.
Clicking on either of them will toggle visibility of the password.  If
you don't want to show the password for the first operation but you do
want to show it for the second operation, then when the second operation
terminates you will see the password for the first one.  Same for typing
TAB in the minibuffer.  Resetting 'read-passwd--hide-password' when
terminating any read password operation might fix that.

Likely password visibility should be a property of the specific reading
process (aka as buffer-local value of 'read-passwd--hide-password') and
not something global.  But then we would also have to change the first
icon of the mode line too and doing that is more tricky.  Currently, all
eye icons are the same because 'read-passwd--mode-line-icon' has the
same value for them.

Probably it might be better to have only one eye icon on any mode line
informing about the visibility of the innermost minibuffer's password
status.  But this becomes tricky when terminating a password operation
because we would have to internally (on the C-level) scan all
minibuffers to find out whether there's another password operation in
progress and remove the eye icon only if there's none.

Otherwise we could try giving only the first eye icon the dynamic eval
property and have all other icons statically have the hide password
property.  Less tricky - we'd only have to hide a password when asking
for another one.

Obviously, we could also inhibit asking for a second password.  But I
suppose this was always possible and would imply an incompatible change.

If anyone has a good idea what to do here, please speak up.

Thanks, martin

[-- Attachment #2: read-passwd.diff --]
[-- Type: text/x-patch, Size: 1857 bytes --]

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index 90b58f560c0..f71393721cb 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -2474,7 +2474,7 @@ read-passwd--mode-line-icon
   "Propertized mode line icon for showing/hiding passwords.")
 
 (defvar read-passwd--hide-password t
-  "Toggle whether password should be hidden in minubuffer.")
+  "Toggle whether password should be hidden in minibuffer.")
 
 (defun read-passwd--hide-password ()
   "Make password in minibuffer hidden or visible."
@@ -2534,13 +2534,14 @@ read-passwd-mode
   :keymap read-passwd-map
   :version "30.1"
 
-  (setq read-passwd--hide-password nil
-        ;; Stolen from `eldoc-minibuffer-message'.
-        read-passwd--mode-line-buffer
-        (window-buffer
-         (or (window-in-direction 'above (minibuffer-window))
-	     (minibuffer-selected-window)
-	     (get-largest-window))))
+  (setq read-passwd--hide-password nil)
+  ;; Stolen from `eldoc-minibuffer-message'.
+  (unless (buffer-live-p read-passwd--mode-line-buffer)
+    (setq read-passwd--mode-line-buffer
+          (window-buffer
+           (or (window-in-direction 'above (minibuffer-window))
+	       (minibuffer-selected-window)
+	       (get-largest-window)))))
 
   (if read-passwd-mode
       (with-current-buffer read-passwd--mode-line-buffer
@@ -2551,8 +2552,10 @@ read-passwd-mode
 	              mode-line-format))))
     (with-current-buffer read-passwd--mode-line-buffer
       ;; Remove `read-passwd--mode-line-icon'.
-      (when (listp mode-line-format)
-        (setq mode-line-format (cdr mode-line-format)))))
+      (when (equal (car mode-line-format)
+		   '(:eval read-passwd--mode-line-icon))
+        (setq mode-line-format (cdr mode-line-format))))
+    (setq read-passwd--mode-line-buffer nil))
 
   (when read-passwd-mode
     (read-passwd-toggle-visibility)))

bug#73768: 'read-passwd-mode' can clobber user defined mode lines

[Eli Zaretskii]

> Cc: 73768@debbugs.gnu.org
> Date: Sun, 13 Oct 2024 16:19:13 +0200
> From:  Michael Albinus via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Please install your patch (you might ask Eli whether it is good for the
> emacs-30 branch).

If it solves the problem, it's okay for emacs-30.

Thanks.