Am 17.05.2012 04:19, schrieb Christopher Faylor:
On Thu, May 17, 2012 at 01:20:06AM +0200, Mikulas Patocka wrote:
On Thu, 17 May 2012, Mikulas Patocka wrote:

Corinna Vinschen:

2012-04-03  Thomas Wolff

        * fhandler.h (class dev_console): Two flags for extended mouse modes.
        * fhandler_console.cc (fhandler_console::read): Implemented
        extended mouse modes 1015 (urxvt, mintty, xterm) and 1006 (xterm).
        Not implemented extended mouse mode 1005 (xterm, mintty).
        Supporting mouse coordinates greater than 222 (each axis).
        Also: two { wrap formatting consistency fixes.
        (fhandler_console::char_command) Initialization of enhanced
        mouse reporting modes.

Patch applied with changes.  Please use __small_sprintf rather than
sprintf.  I also changed the CHangeLog entry slightly.  Keep it short
and in present tense.
Hi

The change (sprintf ->  __small_sprintf) that Corinna applied actually
broke mouse reporting. It is broken in Cygwin 1.7.14 and 1.7.15.

When the user clicks with the first button, the mouse down event is
reported incorrectly, the mouse down event always looks like this:
  1b 5b([) 4d(M) 30(0) 78(x) 32(2)
Note that there is 0x30 (instead of 0x20) as the button. And there are
always fixed coordinates (0x78, 0x32), regardless of where the user
clicks.

This bug breaks the Links textmode browser (if you click on the top line,
you should see the menu, but you don't with Cygwin 1.7.14 and 1.7.15). It
also break Midnight Commander (if you start it with "TERM=xterm mc") and
all other programs that use xterm-style mouse reporting.

The reason is that __small_sprintf and sprintf aren't equivalent.
__small_sprintf processes '%c' format string differently from sprintf. A
piece of code from smallprint.cc:

case 'c':
   {
     int c = va_arg (ap, int);
     if (c>  ' '&&  c<= 127)
       *dst++ = c;
     else
       {
         *dst++ = '0';
         *dst++ = 'x';
         dst = __rn (dst, 16, 0, c, len, pad, LMASK);
       }
   }
   break;

We see that if the character is outside the range 0x21..0x7f,
__small_sprintf prints 0x and the hex value. On the other hand, sprintf
copies the byte unchanged.

__small_sprintf("%c", 32) doesn't print space, it prints 0x20 --- and this
breaks mouse click reporting. It also breaks the extended coordinate
reporting implemented by Thomas because it need to print characters>=
0x80.

The attached patch fixes the mouse bug by changing __small_sprintf back to
sprintf. Alternatively, you can fix __small_sprintf to process "%c"
consistently with sprintf, but I don't know how much other code is
dependent on the current peculiar "%c" processing. So it's safer to change
mouse reporting calls to use sprintf.
I don't know where that odd code came from.  It's apparently been in
smallprint.{c,cc} forever.  I'm nuking it.  I see one potential problem
in fhandler_socket::bind.  It won't be hard to work around if that
function was really relying on it.

Thanks for tracking this down.  The change will be in the next snapshot.
And thanks for analyzing this show-stopper for my mouse reporting patch; I should have noticed myself...
Thomas

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Reply via email to