I've been wondering for a while if this change was necessary; it turns
out it is, a test case is mentioned in the commit message.
Peter Hutterer wrote:
>> Thomas Jaeger wrote:
>>> I can't see any reason why we would treat buttons > 5 differently. This
>>> patch simplifies client code by eliminating the need to call XGrabDevice
>>> after a button has been pressed and prevents race conditions that could
>>> result from that.
>
>
> Following up on that:
> Both the core protocol and the XI protocol spec state that a passive grab is
> to be released when all buttons are logically up, regardless of modifiers.
> This is where the magic number 5 comes in. Core allows for 5 buttons, XI
> allows for more.
>
> This puts us in a difficult position. Core requires us to release the grab if,
> say, only button 6 is down. XI requires us to maintain the grab.
> I think we need a more complex solution than the one you proposed: test
> whether the passive grab is a core or an XI grab and release depending on this
> condition.
> Something like
> if ((grab->coreGrab && !button->state) && ... ||
> (!grab->coreGrab && AllButtonsAreUp(b))
>From 3f8ba578ad18b7135031197f6ec5145afcd1479a Mon Sep 17 00:00:00 2001
From: Thomas Jaeger <thjae...@gmail.com>
Date: Mon, 22 Dec 2008 00:55:09 +0100
Subject: [PATCH] Count the number of logically down buttons in buttonsDown
This fixes the following bug. Assuming your window manager grabs
Alt+Button1 to move windows, map Button3 to 0 via XSetPointerMapping,
then press the physical button 3 (this shouldn't have any effect), press
Alt and then button 1. The press event is delivered to the application
instead of firing the grab.
---
Xi/exevents.c | 8 ++++----
include/inputstr.h | 6 +++++-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/Xi/exevents.c b/Xi/exevents.c
index 2aa3161..b4359a8 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -895,10 +895,10 @@ UpdateDeviceState(DeviceIntPtr device, xEvent* xE, int count)
*kptr |= bit;
if (device->valuator)
device->valuator->motionHintWindow = NullWindow;
- b->buttonsDown++;
- b->motionMask = DeviceButtonMotionMask;
if (!b->map[key])
return DONT_PROCESS;
+ b->buttonsDown++;
+ b->motionMask = DeviceButtonMotionMask;
if (b->map[key] <= 5)
b->state |= (Button1Mask >> 1) << b->map[key];
SetMaskForEvent(device->id, Motion_Filter(b), DeviceMotionNotify);
@@ -927,10 +927,10 @@ UpdateDeviceState(DeviceIntPtr device, xEvent* xE, int count)
*kptr &= ~bit;
if (device->valuator)
device->valuator->motionHintWindow = NullWindow;
- if (b->buttonsDown >= 1 && !--b->buttonsDown)
- b->motionMask = 0;
if (!b->map[key])
return DONT_PROCESS;
+ if (b->buttonsDown >= 1 && !--b->buttonsDown)
+ b->motionMask = 0;
if (b->map[key] <= 5)
b->state &= ~((Button1Mask >> 1) << b->map[key]);
SetMaskForEvent(device->id, Motion_Filter(b), DeviceMotionNotify);
diff --git a/include/inputstr.h b/include/inputstr.h
index 4719d37..515b6aa 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -185,7 +185,11 @@ typedef struct _ValuatorClassRec {
typedef struct _ButtonClassRec {
CARD8 numButtons;
- CARD8 buttonsDown; /* number of buttons currently down */
+ CARD8 buttonsDown; /* number of buttons currently down
+ This counts logical buttons, not
+ physical ones, i.e if some buttons
+ are mapped to 0, they're not counted
+ here */
unsigned short state;
Mask motionMask;
CARD8 down[DOWN_LENGTH];
--
1.6.0.4
_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg
