This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] fix broken mouse in Cygwin 1.7.14 and 1.7.15


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]