Button mapping mis-applied

[Keith Packard]

I was debugging a DGA/X mouse issue -- buttons pressed in DGA would not
be released if DGA terminated before the button was released -- and
found an actual X-only bug. 

I remap my mouse buttons so that the left button is button 3 and the
right button is button 1 ('left handed'):

$  xmodmap -pp
There are 32 pointer buttons defined.

    Physical        Button
     Button          Code
        1              3
        2              2
        3              1
        4              4
        5              5
	      ...

This mapping is currently managed in GetPointerEvents:

(around line 1033):
	kbp->detail = pDev->button->map[buttons]

However, UpdateDeviceState *does the mapping again* when computing the
state:

        if (!b->map[key])
            return DONT_PROCESS;
        if (b->map[key] <= 5)
            b->state |= (Button1Mask >> 1) << b->map[key];

So, in my environment, the right button (button 1) would set Button3Mask
instead of Button1Mask in the state field. Oops.

While it might make sense to just fix up UpdateDeviceState, it turns out
to be better to leave the raw button number in the event because DGA
reports raw device events without button translation, and so when it
calls UpdateDeviceState, the button down counts get scrambled and
buttons get stuck down.

Here's the code that deals with leaving the raw button number in the
event:

diff --git a/Xi/exevents.c b/Xi/exevents.c
index 725c578..57fb65b 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -1009,6 +1009,7 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, int count)
     if (device->isMaster)
         CheckMotion(xE, device);
 
+    key = xE->u.u.detail;
     if (xE->u.u.type != DeviceValuator && xE->u.u.type != GenericEvent) {
 	GetSpritePosition(device, &rootX, &rootY);
 	xE->u.keyButtonPointer.rootX = rootX;
@@ -1017,7 +1018,6 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, int count)
 
         xE->u.keyButtonPointer.state = state;
 
-        key = xE->u.u.detail;
     }
     if (DeviceEventCallback) {
 	DeviceEventInfoRec eventinfo;
@@ -1066,9 +1066,11 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, int count)
             (key == device->deviceGrab.activatingKey))
 	    deactivateDeviceGrab = TRUE;
     } else if (xE->u.u.type == DeviceButtonPress) {
-	xE->u.u.detail = key;
-	if (xE->u.u.detail == 0)
+	xE->u.u.detail = b->map[key];
+	if (xE->u.u.detail == 0) {
+	    xE->u.u.detail = key;
 	    return;
+	}
         if (!grab && CheckDeviceGrabs(device, xE, 0, count))
         {
             /* if a passive grab was activated, the event has been sent
@@ -1077,9 +1079,11 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, int count)
         }
 
     } else if (xE->u.u.type == DeviceButtonRelease) {
-	xE->u.u.detail = key;
-	if (xE->u.u.detail == 0)
+	xE->u.u.detail = b->map[key];
+	if (xE->u.u.detail == 0) {
+	    xE->u.u.detail = key;
 	    return;
+	}
         if (!b->state && device->deviceGrab.fromPassiveGrab)
             deactivateDeviceGrab = TRUE;
     }
@@ -1094,6 +1098,7 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, int count)
 
     if (deactivateDeviceGrab == TRUE)
 	(*device->deviceGrab.DeactivateGrab) (device);
+    xE->u.u.detail = key;
 }
 
 _X_EXPORT int
diff --git a/dix/getevents.c b/dix/getevents.c
index 5227c54..b00bc11 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -1030,7 +1030,7 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, int type, int buttons,
             kbp->type = DeviceButtonPress;
         else if (type == ButtonRelease)
             kbp->type = DeviceButtonRelease;
-        kbp->detail = pDev->button->map[buttons];
+        kbp->detail = buttons;
     }
 
     kbp->root_x = cx; /* root_x/y always in screen coords */


-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20080614/4c88fbcb/attachment.pgp>