[dev] [st] [PATCH] Fix some bugs in mouse tracking logic

From: Michael Forney <mforney_AT_mforney.org>
Date: Thu, 25 Jul 2013 16:19:19 -0700

* Button number in X10 mode:

  I believe the button - 1 came from "C b is button - 1" from [0].
  However, above this section, it states

    "Normally, parameters (such as pointer poisition and button number)
     for all mouse tracking escape sequences generated by xterm encode
     numeric parameters in a single character as value+32. For example, !
     specifies the value 1."

  Also, from the description of SGR,

    "The encoded button value in this case does not add 32 since that
     was useful only in the X10 scheme for ensuring that the byte
     containing the button value is a printable code."

  This suggests that we should still add 32 to the button value when in
  MODE_MOUSEX10.

* No button release reporting in X10 mode:

    "X10 compatibility mode sends an escape sequence only on button press,
     encoding the location and the mouse button pressed."

* Fix MODE_MOUSEMOTION:

  Currently, motion reporting is skipped when oldbutton == 3
  (corresponding to no button being pressed). However, oldbutton is
  only set on a button press, which will never be 3.

[0]: http://invisible-island.net/xterm/ctlseqs/ctlseqs.html

---
I came across these when porting st to wayland (if you're interested in that,
watch the wayland mailing list).
I haven't actually run into any of these bugs (I don't use terminal mouse
support), but these changes seem correct to me based on [0].
Any feedback or testing is welcome!
 st.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/st.c b/st.c
index 947373f..8992998 100644
--- a/st.c
+++ b/st.c
_AT_@ -818,18 +818,23 @@ mousereport(XEvent *e) {
 		button = oldbutton + 32;
 		ox = x;
 		oy = y;
-	} else if(!IS_SET(MODE_MOUSESGR)
-			&& (e->xbutton.type == ButtonRelease
-				|| button == AnyButton)) {
-		button = 3;
 	} else {
-		button -= Button1;
-		if(button >= 3)
-			button += 64 - 3;
+		if(!IS_SET(MODE_MOUSESGR) && e->xbutton.type == ButtonRelease) {
+			button = 3;
+		} else {
+			button -= Button1;
+			if(button >= 3)
+				button += 64 - 3;
+		}
 		if(e->xbutton.type == ButtonPress) {
 			oldbutton = button;
 			ox = x;
 			oy = y;
+		} else if(e->xbutton.type == ButtonRelease) {
+			oldbutton = 3;
+			/* MODE_MOUSEX10: no button release reporting */
+			if(IS_SET(MODE_MOUSEX10))
+				return;
 		}
 	}
 
_AT_@ -846,8 +851,7 @@ mousereport(XEvent *e) {
 				e->xbutton.type == ButtonRelease ? 'm' : 'M');
 	} else if(x < 223 && y < 223) {
 		len = snprintf(buf, sizeof(buf), "\033[M%c%c%c",
-				IS_SET(MODE_MOUSEX10)? button-1 : 32+button,
-				32+x+1, 32+y+1);
+				32+button, 32+x+1, 32+y+1);
 	} else {
 		return;
 	}
-- 
1.8.3

Received on Fri Jul 26 2013 - 01:19:19 CEST