Possible emacs bug when type password fields

Possible emacs bug when type password fields

[Borja Tarraso Hueso]

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

Hi,

I was wondering if this is a known issue or not, but it seems some emacs
bug, when I tried M-x gnus or M-x erc or any other feature in emacs (it
means that is not emacs specific feature bug) I cannot use num keypad to
type the password field. I tried in two different computers, with two
different emacs versions, and different features, with same results.

Emacs version affected:

GNU Emacs 23.0.90.2 (i686-pc-linux-gnu) of 2009-04-21 on blackhole

Steps to reproduce the issue:

Way 1:
1) M-x gnus
2) when password prompt appears, try to use numkey pad.
3) see that dots do not appears for any key pressed.

Way 2:
1) M-x erc
2) ask several cuestions about irc server, port, nickname.
3) when password prompt appears, try to use numkey pad.
4) see that dots do not appears for any key pressed.

I did not test any other emacs feature that shows a prompt password.

Thanks!

Borja Tarraso

[-- Attachment #2: Type: text/html, Size: 984 bytes --]

Re: Possible emacs bug when type password fields

[Harald Hanche-Olsen]

+ Borja Tarraso Hueso <borja.tarraso.hueso@googlemail.com>:

> I was wondering if this is a known issue or not, but it seems some emacs
> bug, when I tried M-x gnus or M-x erc or any other feature in emacs (it
> means that is not emacs specific feature bug) I cannot use num keypad to
> type the password field. I tried in two different computers, with two
> different emacs versions, and different features, with same results.

It is more easily reproduced by typing

  M-: (read-passwd "Try it: ") RET

and typing some characters from the numeric keypad.

The problem seems to be that read-passwd does its own event processing,
and it doesn't recognize key translations such as kp-1 -> 1, etc.
(Type C-h k followed by 1 on the numeric keypad to see what I mean.)

(I see this with the x11 window system, not with nextstep, where that
translation happens at the OS level. Quite possibly, you don't see it
on windows either.)

More of a misfeature than a bug perhaps, but it would be worth fixing
if it's not too much trouble.

- Harald

Re: Possible emacs bug when type password fields

[Stefan Monnier]

>> I was wondering if this is a known issue or not, but it seems some emacs
>> bug, when I tried M-x gnus or M-x erc or any other feature in emacs (it
>> means that is not emacs specific feature bug) I cannot use num keypad to
>> type the password field. I tried in two different computers, with two
>> different emacs versions, and different features, with same results.

Can you try the patch below?  I don't have any keyboard with a numeric
keypad any more ;-)

Note that this patch is not acceptable for Emacs-23.1 (which is now
too far in the pretest phase for such a change).


        Stefan


--- subr.el.~1.636.~	2009-04-20 12:12:55.000000000 -0400
+++ subr.el	2009-05-01 15:52:48.000000000 -0400
@@ -1728,6 +1728,47 @@
 \f
 ;;;; Input and display facilities.
 
+(defconst read-key-empty-map (make-sparse-keymap))
+(defvar read-key-delay 0.1)
+
+(defun read-key (&optional prompt)
+  "Read a key from the keyboard.
+Contrary to `read-event' this will not return a raw event but instead will
+obey the input decoding and translations usually done by `read-key-sequence'.
+So escape sequences and keyboard encoding are taken into account.
+When there's an ambiguity because the key looks like the prefix of
+some sort of escape sequence, the ambiguity is resolved via `read-key-delay'."
+  (let ((overriding-terminal-local-map read-key-empty-map)
+	(overriding-local-map nil)
+	(old-global-map (current-global-map))
+        (timer (run-with-idle-timer
+                ;; Wait long enough that Emacs has the time to receive and
+                ;; process all the raw events associated with the single-key.
+                ;; But don't wait too long, or the user may find the delay
+                ;; annoying (or keep hitting more keys which may then get
+                ;; lost or misinterpreted).
+                ;; This is only relevant for keys which Emacs perceives as
+                ;; "prefixes", such as C-x (because of the C-x 8 map in
+                ;; key-translate-table and the C-x @ map in function-key-map)
+                ;; or ESC (because of terminal escape sequences in
+                ;; input-decode-map).
+                read-key-delay t
+                (lambda ()
+                  (let ((keys (this-command-keys-vector)))
+                    (unless (zerop (length keys))
+                      ;; `keys' is non-empty, so the user has hit at least
+                      ;; one key; there's no point waiting any longer, even
+                      ;; though read-key-sequence thinks we should wait
+                      ;; for more input to decide how to interpret the
+                      ;; current input.
+                      (throw 'read-key keys)))))))
+    (unwind-protect
+        (progn
+	  (use-global-map read-key-empty-map)
+	  (aref	(catch 'read-key (read-key-sequence prompt nil t)) 0))
+      (cancel-timer timer)
+      (use-global-map old-global-map))))
+
 (defvar read-quoted-char-radix 8
   "*Radix for \\[quoted-insert] and other uses of `read-quoted-char'.
 Legitimate radix values are 8, 10 and 16.")
@@ -1844,10 +1885,7 @@
 	(while (progn (message "%s%s"
 			       prompt
 			       (make-string (length pass) ?.))
-		      ;; We used to use read-char-exclusive, but that
-		      ;; gives funny behavior when the user presses,
-		      ;; e.g., the arrow keys.
-		      (setq c (read-event nil t))
+		      (setq c (read-key))
 		      (not (memq c stop-keys)))
 	  (clear-this-command-keys)
 	  (cond ((memq c rubout-keys) ; rubout

Re: Possible emacs bug when type password fields

[Harald Hanche-Olsen]

+ Stefan Monnier <monnier@IRO.UMontreal.CA>:

> >> I cannot use num keypad to type the password field.
> 
> Can you try the patch below?  I don't have any keyboard with a numeric
> keypad any more ;-)

It appears to do the trick, though I haven't gone out of my way to
think of things that might break it.

- Harald

Re: Possible emacs bug when type password fields

[Borja Tarraso]

Hi Stefan,

Sorry for late reply. I patched using your diffs, and it works, however
i seen new issues, e.g. when I press C-g in this input password is
not cancelling this command and writes some additional characters
to the password field.

Thanks!

Borja Tarraso

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> I was wondering if this is a known issue or not, but it seems some emacs
>>> bug, when I tried M-x gnus or M-x erc or any other feature in emacs (it
>>> means that is not emacs specific feature bug) I cannot use num keypad to
>>> type the password field. I tried in two different computers, with two
>>> different emacs versions, and different features, with same results.
>
> Can you try the patch below?  I don't have any keyboard with a numeric
> keypad any more ;-)
>
> Note that this patch is not acceptable for Emacs-23.1 (which is now
> too far in the pretest phase for such a change).
>
>
>         Stefan
>
>
> --- subr.el.~1.636.~	2009-04-20 12:12:55.000000000 -0400
> +++ subr.el	2009-05-01 15:52:48.000000000 -0400
> @@ -1728,6 +1728,47 @@
>  \f
>  ;;;; Input and display facilities.
>  
> +(defconst read-key-empty-map (make-sparse-keymap))
> +(defvar read-key-delay 0.1)
> +
> +(defun read-key (&optional prompt)
> +  "Read a key from the keyboard.
> +Contrary to `read-event' this will not return a raw event but instead will
> +obey the input decoding and translations usually done by `read-key-sequence'.
> +So escape sequences and keyboard encoding are taken into account.
> +When there's an ambiguity because the key looks like the prefix of
> +some sort of escape sequence, the ambiguity is resolved via `read-key-delay'."
> +  (let ((overriding-terminal-local-map read-key-empty-map)
> +	(overriding-local-map nil)
> +	(old-global-map (current-global-map))
> +        (timer (run-with-idle-timer
> +                ;; Wait long enough that Emacs has the time to receive and
> +                ;; process all the raw events associated with the single-key.
> +                ;; But don't wait too long, or the user may find the delay
> +                ;; annoying (or keep hitting more keys which may then get
> +                ;; lost or misinterpreted).
> +                ;; This is only relevant for keys which Emacs perceives as
> +                ;; "prefixes", such as C-x (because of the C-x 8 map in
> +                ;; key-translate-table and the C-x @ map in function-key-map)
> +                ;; or ESC (because of terminal escape sequences in
> +                ;; input-decode-map).
> +                read-key-delay t
> +                (lambda ()
> +                  (let ((keys (this-command-keys-vector)))
> +                    (unless (zerop (length keys))
> +                      ;; `keys' is non-empty, so the user has hit at least
> +                      ;; one key; there's no point waiting any longer, even
> +                      ;; though read-key-sequence thinks we should wait
> +                      ;; for more input to decide how to interpret the
> +                      ;; current input.
> +                      (throw 'read-key keys)))))))
> +    (unwind-protect
> +        (progn
> +	  (use-global-map read-key-empty-map)
> +	  (aref	(catch 'read-key (read-key-sequence prompt nil t)) 0))
> +      (cancel-timer timer)
> +      (use-global-map old-global-map))))
> +
>  (defvar read-quoted-char-radix 8
>    "*Radix for \\[quoted-insert] and other uses of `read-quoted-char'.
>  Legitimate radix values are 8, 10 and 16.")
> @@ -1844,10 +1885,7 @@
>  	(while (progn (message "%s%s"
>  			       prompt
>  			       (make-string (length pass) ?.))
> -		      ;; We used to use read-char-exclusive, but that
> -		      ;; gives funny behavior when the user presses,
> -		      ;; e.g., the arrow keys.
> -		      (setq c (read-event nil t))
> +		      (setq c (read-key))
>  		      (not (memq c stop-keys)))
>  	  (clear-this-command-keys)
>  	  (cond ((memq c rubout-keys) ; rubout

Re: Possible emacs bug when type password fields

[Stefan Monnier]

> Sorry for late reply. I patched using your diffs, and it works, however
> i seen new issues, e.g. when I press C-g in this input password is
> not cancelling this command and writes some additional characters
> to the password field.

Thanks.  The additional patch below should fix this (and another
potential problem as well if you hit keys like down/up/...).


        Stefan


=== modified file 'lisp/subr.el'
--- lisp/subr.el	2009-05-02 00:32:01 +0000
+++ lisp/subr.el	2009-05-06 01:21:02 +0000
@@ -1997,10 +1997,11 @@
 		     (and (arrayp pass) (clear-string pass))
 		     (setq pass new-pass))))
 		((not (numberp c)))
-		((= c ?\C-u) ; kill line
+		((eq c ?\C-u) ; kill line
 		 (and (arrayp pass) (clear-string pass))
 		 (setq pass ""))
-		((= c ?\C-y) ; yank
+                ((eq c ?\C-g) (signal 'quit nil))
+		((eq c ?\C-y) ; yank
 		 (let* ((str (condition-case nil
 				 (current-kill 0)
 			       (error nil)))

Re: Possible emacs bug when type password fields

[Andreas Schwab]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> --- lisp/subr.el	2009-05-02 00:32:01 +0000
> +++ lisp/subr.el	2009-05-06 01:21:02 +0000
> @@ -1997,10 +1997,11 @@
>  		     (and (arrayp pass) (clear-string pass))
>  		     (setq pass new-pass))))
>  		((not (numberp c)))
> -		((= c ?\C-u) ; kill line
> +		((eq c ?\C-u) ; kill line

That should not make a difference, since c is guaranteed to be a number
here.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Possible emacs bug when type password fields

[Stefan Monnier]

>> --- lisp/subr.el	2009-05-02 00:32:01 +0000
>> +++ lisp/subr.el	2009-05-06 01:21:02 +0000
>> @@ -1997,10 +1997,11 @@
>> (and (arrayp pass) (clear-string pass))
>> (setq pass new-pass))))
>> ((not (numberp c)))
>> -		((= c ?\C-u) ; kill line
>> +		((eq c ?\C-u) ; kill line

> That should not make a difference, since c is guaranteed to be a number
> here.

That's the thing: by changing red-char into read-key (done by the
previous patch), it is not guaranteed any more.


        Stefan

Re: Possible emacs bug when type password fields

[Harald Hanche-Olsen]

+ Stefan Monnier <monnier@iro.umontreal.ca>:

> >> --- lisp/subr.el	2009-05-02 00:32:01 +0000
> >> +++ lisp/subr.el	2009-05-06 01:21:02 +0000
> >> @@ -1997,10 +1997,11 @@
> >> (and (arrayp pass) (clear-string pass))
> >> (setq pass new-pass))))
> >> ((not (numberp c)))
> >> -		((= c ?\C-u) ; kill line
> >> +		((eq c ?\C-u) ; kill line
> 
> > That should not make a difference, since c is guaranteed to be a number
> > here.
> 
> That's the thing: by changing red-char into read-key (done by the
> previous patch), it is not guaranteed any more.

I don't get it. How is c not guaranteed to be a number in a cond
clause following the clause

  		((not (numberp c)))

?

- Harald

Re: Possible emacs bug when type password fields

[Andreas Schwab]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> --- lisp/subr.el	2009-05-02 00:32:01 +0000
>>> +++ lisp/subr.el	2009-05-06 01:21:02 +0000
>>> @@ -1997,10 +1997,11 @@
>>> (and (arrayp pass) (clear-string pass))
>>> (setq pass new-pass))))
>>> ((not (numberp c)))
>>> -		((= c ?\C-u) ; kill line
>>> +		((eq c ?\C-u) ; kill line
>
>> That should not make a difference, since c is guaranteed to be a number
>> here.
>
> That's the thing: by changing red-char into read-key (done by the
> previous patch), it is not guaranteed any more.

Look closer.  The preceeding clause catches all non-numberp cases.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Possible emacs bug when type password fields

[Stefan Monnier]

>>> That should not make a difference, since c is guaranteed to be a number
>>> here.
>> That's the thing: by changing red-char into read-key (done by the
>> previous patch), it is not guaranteed any more.
> Look closer.  The preceeding clause catches all non-numberp cases.

Duh!


        Stef

Re: Possible emacs bug when type password fields

[Borja Tarraso]

Hi Stefan,

Near :-) this time works fine, i can write from the beginning, however
C-g stills does not work, when I press C-g it writes another additional
character.

Thanks!

Borja Tarraso

Andreas Schwab <schwab@linux-m68k.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>>> --- lisp/subr.el	2009-05-02 00:32:01 +0000
>>>> +++ lisp/subr.el	2009-05-06 01:21:02 +0000
>>>> @@ -1997,10 +1997,11 @@
>>>> (and (arrayp pass) (clear-string pass))
>>>> (setq pass new-pass))))
>>>> ((not (numberp c)))
>>>> -		((= c ?\C-u) ; kill line
>>>> +		((eq c ?\C-u) ; kill line
>>
>>> That should not make a difference, since c is guaranteed to be a number
>>> here.
>>
>> That's the thing: by changing red-char into read-key (done by the
>> previous patch), it is not guaranteed any more.
>
> Look closer.  The preceeding clause catches all non-numberp cases.
>
> Andreas.

Re: Possible emacs bug when type password fields

[Stefan Monnier]

> Near :-) this time works fine, i can write from the beginning, however
> C-g stills does not work, when I press C-g it writes another additional
> character.

Hmm... this is odd, I cannot reproduce it.  Can you try and single-step
through that code to see why the (eq c ?\C-g) test fails?
To do that, just move point to inside the function and hit C-u M-C-x
(which marks the function for interactive debugging) and then do
M-: (read-password "toto: ").
Once in the debugger, use `n' or SPC to advance through the code.


        Stefan

Re: Possible emacs bug when type password fields

[Borja Tarraso]

Hi Stefan,

I was looking at the code with some friend. Adding some extra condition
to catch C-g problem is fixed.

The problem was in read-key function; this function does not control C-g as
it is using read-key-sequence (as documentation is menction: "A C-g
typed while in this function is treated like any other character, and
`quit-flag' is not set") so it explains why this is happening, as
is not reading each key and inserting, instead of that, it allows to the
user writing on that buffer and obtaining the content later.

Probably lots of things have dependency of read-key, for this reason it
was added this extra condition... but i don't know really if it's the
best solution.


==> Full diffs:


*** emacs/lisp/subr.el	2009-02-14 09:06:56.000000000 +0000
--- emacs_hack/lisp/subr.el	2009-05-14 01:29:52.000000000 +0100
***************
*** 1721,1726 ****
--- 1721,1767 ----
  

  ;;;; Input and display facilities.
  
+ (defconst read-key-empty-map (make-sparse-keymap))
+ (defvar read-key-delay 0.1)
+ 
+ (defun read-key (&optional prompt)
+   "Read a key from the keyboard.
+ Contrary to `read-event' this will not return a raw event but instead will
+ obey the input decoding and translations usually done by `read-key-sequence'.
+ So escape sequences and keyboard encoding are taken into account.
+ When there's an ambiguity because the key looks like the prefix of
+ some sort of escape sequence, the ambiguity is resolved via `read-key-delay'."
+   (let ((overriding-terminal-local-map read-key-empty-map)
+ 	(overriding-local-map nil)
+ 	(old-global-map (current-global-map))
+         (timer (run-with-idle-timer
+                 ;; Wait long enough that Emacs has the time to receive and
+                 ;; process all the raw events associated with the single-key.
+                 ;; But don't wait too long, or the user may find the delay
+                 ;; annoying (or keep hitting more keys which may then get
+                 ;; lost or misinterpreted).
+                 ;; This is only relevant for keys which Emacs perceives as
+                 ;; "prefixes", such as C-x (because of the C-x 8 map in
+                 ;; key-translate-table and the C-x @ map in function-key-map)
+                 ;; or ESC (because of terminal escape sequences in
+                 ;; input-decode-map).
+                 read-key-delay t
+                 (lambda ()
+                   (let ((keys (this-command-keys-vector)))
+                     (unless (zerop (length keys))
+                       ;; `keys' is non-empty, so the user has hit at least
+                       ;; one key; there's no point waiting any longer, even
+                       ;; though read-key-sequence thinks we should wait
+                       ;; for more input to decide how to interpret the
+                       ;; current input.
+                       (throw 'read-key keys)))))))
+     (unwind-protect
+         (progn
+ 	  (use-global-map read-key-empty-map)
+ 	  (aref	(catch 'read-key (read-key-sequence prompt nil t)) 0))
+       (cancel-timer timer)
+       (use-global-map old-global-map))))
+ 
  (defvar read-quoted-char-radix 8
    "*Radix for \\[quoted-insert] and other uses of `read-quoted-char'.
  Legitimate radix values are 8, 10 and 16.")
***************
*** 1837,1846 ****
  	(while (progn (message "%s%s"
  			       prompt
  			       (make-string (length pass) ?.))
! 		      ;; We used to use read-char-exclusive, but that
! 		      ;; gives funny behavior when the user presses,
! 		      ;; e.g., the arrow keys.
! 		      (setq c (read-event nil t))
  		      (not (memq c stop-keys)))
  	  (clear-this-command-keys)
  	  (cond ((memq c rubout-keys) ; rubout
--- 1878,1884 ----
  	(while (progn (message "%s%s"
  			       prompt
  			       (make-string (length pass) ?.))
! 		      (setq c (read-key))
  		      (not (memq c stop-keys)))
  	  (clear-this-command-keys)
  	  (cond ((memq c rubout-keys) ; rubout
***************
*** 1848,1855 ****
  		   (let ((new-pass (substring pass 0 -1)))
  		     (and (arrayp pass) (clear-string pass))
  		     (setq pass new-pass))))
  		((not (numberp c)))
! 		((= c ?\C-u) ; kill line
  		 (and (arrayp pass) (clear-string pass))
  		 (setq pass ""))
  		((= c ?\C-y) ; yank
--- 1886,1895 ----
  		   (let ((new-pass (substring pass 0 -1)))
  		     (and (arrayp pass) (clear-string pass))
  		     (setq pass new-pass))))
+ 		((eq c ?\C-g)
+ 		 (keyboard-quit))
  		((not (numberp c)))
! 		((eq c ?\C-u) ; kill line
  		 (and (arrayp pass) (clear-string pass))
  		 (setq pass ""))
  		((= c ?\C-y) ; yank


==> End.

Thanks!

Borja Tarraso

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Near :-) this time works fine, i can write from the beginning, however
>> C-g stills does not work, when I press C-g it writes another additional
>> character.
>
> Hmm... this is odd, I cannot reproduce it.  Can you try and single-step
> through that code to see why the (eq c ?\C-g) test fails?
> To do that, just move point to inside the function and hit C-u M-C-x
> (which marks the function for interactive debugging) and then do
> M-: (read-password "toto: ").
> Once in the debugger, use `n' or SPC to advance through the code.
>
>
>         Stefan