From 1884db430a5680e37e94726dff46686e2218d525 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 24 Jun 2010 12:52:53 +1000 Subject: [PATCH 1/7] Revert "dix: use the event mask of the grab for TryClientEvents." Behaviour of earlier X servers was to deliver the ButtonPress event unconditionally, regardless of the actual event mask being set. This is documented in the protocol: "This request establishes a passive grab. In the future, the pointer is actively grabbed as described in GrabPointer, the last-pointer-grab time is set to the time at which the button was pressed (as transmitted in the ButtonPress event), and the ButtonPress event is reported if all of the following conditions are true: " Thus, a GrabButton event will always deliver the button press event, a GrabKey always the key press event, etc. Same goes for XI and XI2. Reproducible with a simple client requesting a button grab in the form of: XGrabButton(dpy, AnyButton, AnyModifier, win, True, ButtonReleaseMask, GrabModeAsync, GrabModeAsync, None, None); On servers before MPX/XI2, the client will receive a button press and release event. On current servers, the client receives only the release. Clients that expect the press event to be delivered unconditionally. XTS Xlib13 XGrabButton 5/39 now passes. This reverts commit 48585bd1e3e98db0f3df1ecc68022510216e00cc. Effectively reverts commit 1c612acca8568fcdf9761d23f112adaf4d496f1b as well, the code introduced with 1c612 is not needed anymore. Conflicts: dix/events.c Signed-off-by: Peter Hutterer Acked-by: Daniel Stone Reviewed-by: Keith Packard --- dix/events.c | 52 ++-------------------------------------------------- 1 file changed, 2 insertions(+), 50 deletions(-) diff --git a/dix/events.c b/dix/events.c index ae9847c93..e1c3d0a02 100644 --- a/dix/events.c +++ b/dix/events.c @@ -3420,7 +3420,6 @@ CheckPassiveGrabsOnWindow( { DeviceIntPtr gdev; XkbSrvInfoPtr xkbi = NULL; - Mask mask = 0; gdev= grab->modifierDevice; if (grab->grabtype == GRABTYPE_CORE) @@ -3535,9 +3534,6 @@ CheckPassiveGrabsOnWindow( } xE = &core; count = 1; - mask = grab->eventMask; - if (grab->ownerEvents) - mask |= pWin->eventMask; } else if (match & XI2_MATCH) { rc = EventToXI2((InternalEvent*)event, &xE); @@ -3549,34 +3545,6 @@ CheckPassiveGrabsOnWindow( continue; } count = 1; - - /* FIXME: EventToXI2 returns NULL for enter events, so - * dereferencing the event is bad. Internal event types are - * aligned with core events, so the else clause is valid. - * long-term we should use internal events for enter/focus - * as well */ - if (xE) - mask = grab->xi2mask[device->id][((xGenericEvent*)xE)->evtype/8]; - else if (event->type == XI_Enter || event->type == XI_FocusIn) - mask = grab->xi2mask[device->id][event->type/8]; - - if (grab->ownerEvents && wOtherInputMasks(grab->window)) - { - InputClientsPtr icp = - wOtherInputMasks(grab->window)->inputClients; - - while(icp) - { - if (rClient(icp) == rClient(grab)) - { - int evtype = (xE) ? ((xGenericEvent*)xE)->evtype : event->type; - mask |= icp->xi2mask[device->id][evtype/8]; - break; - } - - icp = icp->next; - } - } } else { rc = EventToXI((InternalEvent*)event, &xE, &count); @@ -3587,23 +3555,6 @@ CheckPassiveGrabsOnWindow( "(%d, %d).\n", device->name, event->type, rc); continue; } - mask = grab->eventMask; - if (grab->ownerEvents && wOtherInputMasks(grab->window)) - { - InputClientsPtr icp = - wOtherInputMasks(grab->window)->inputClients; - - while(icp) - { - if (rClient(icp) == rClient(grab)) - { - mask |= icp->mask[device->id]; - break; - } - - icp = icp->next; - } - } } (*grabinfo->ActivateGrab)(device, grab, currentTime, TRUE); @@ -3612,7 +3563,8 @@ CheckPassiveGrabsOnWindow( { FixUpEventFromWindow(device, xE, grab->window, None, TRUE); - TryClientEvents(rClient(grab), device, xE, count, mask, + TryClientEvents(rClient(grab), device, xE, count, + GetEventFilter(device, xE), GetEventFilter(device, xE), grab); } From dbf249ec6638f0a8dfa4c2286099845aafc8ac88 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 29 Jun 2010 10:43:51 +1000 Subject: [PATCH 2/7] xkb: remove now obsolete comment. Looks like nothing broke from removing the hardcoded CoreProcessPointerEvent call. Whoop. Di. Doo. Signed-off-by: Peter Hutterer --- xkb/xkbAccessX.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/xkb/xkbAccessX.c b/xkb/xkbAccessX.c index b5486b73b..6d17c75b1 100644 --- a/xkb/xkbAccessX.c +++ b/xkb/xkbAccessX.c @@ -712,22 +712,6 @@ DeviceEvent *event = &ev->device_event; changed |= XkbPointerButtonMask; } - /* Guesswork. mostly. - * xkb actuall goes through some effort to transparently wrap the - * processInputProcs (see XkbSetExtension). But we all love fun, so the - * previous XKB implementation just hardcoded the CPPE call here instead - * of unwrapping like anybody with any sense of decency would do. - * I got no clue what the correct thing to do is, but my guess is that - * it's not hardcoding. I may be wrong. whatever it is, don't come whining - * to me. I just work here. - * - * Anyway. here's the old call, if you don't like the wrapping, revert it. - * - * CoreProcessPointerEvent(xE,mouse,count); - * - * see. it's still steaming. told you. (whot) - */ - UNWRAP_PROCESS_INPUT_PROC(mouse, xkbPrivPtr, backupproc); mouse->public.processInputProc(ev, mouse); COND_WRAP_PROCESS_INPUT_PROC(mouse, xkbPrivPtr, From c7330ecb5d28d7a92d24feb289f7f1812ce055a4 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 30 Jun 2010 13:23:14 +1000 Subject: [PATCH 3/7] dix: fix up erroneous error message. (WW) Device 'device name' has 36 axes, only using first 36. does seem a bit silly. Signed-off-by: Peter Hutterer --- dix/devices.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dix/devices.c b/dix/devices.c index 034d5e6d2..ac5806a28 100644 --- a/dix/devices.c +++ b/dix/devices.c @@ -1239,7 +1239,7 @@ InitValuatorClassDeviceStruct(DeviceIntPtr dev, int numAxes, Atom *labels, if (!dev) return FALSE; - if (numAxes >= MAX_VALUATORS) + if (numAxes > MAX_VALUATORS) { LogMessage(X_WARNING, "Device '%s' has %d axes, only using first %d.\n", From 09645864f5a52882eee51c801b3e610d683e7147 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 29 Jun 2010 13:49:27 +1000 Subject: [PATCH 4/7] xkb: Mark switch case fallthrough with comment. Signed-off-by: Peter Hutterer --- xkb/xkbActions.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c index c5030d04d..d1149a196 100644 --- a/xkb/xkbActions.c +++ b/xkb/xkbActions.c @@ -633,6 +633,8 @@ _XkbFilterPointerBtn( XkbSrvInfoPtr xkbi, break; } xkbi->lockedPtrButtons&= ~(1<device, 0, button); break; From 69ac909878ef80bb74c4a9ca4150eda66debd754 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 29 Jun 2010 12:12:53 +1000 Subject: [PATCH 5/7] xkb: merge lockedPtrButtons state from all attached SDs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: lockedPtrButtons keeps the state of the buttons locked by a PointerKeys button press. Unconditionally clearing the bits may cause stuck buttons in this sequence of events: 1. type Shift + NumLock to enable PointerKeys 2. type 0/Ins on keypad to emulate Button 1 press → button1 press event to client 3. press and release button 1 on physical mouse → button1 release event to client Button 1 on the MD is now stuck and cannot be released. Cause: XKB PointerKeys button events are posted through the XTEST pointer device. Once a press is generated, the XTEST device's button is down. The DIX merges the button state of all attached SDs, hence the MD will have a button down while the XTEST device has a button down. PointerKey button events are only generated on the master device to avoid duplicate events (see XkbFakeDeviceButton()). If the MD has the lockedPtrButtons bit cleared by a release event on a physical device, no such event is generated when a keyboard device triggers the PointerKey ButtonRelease trigger. Since the event - if generated - is posted through the XTEST pointer device, lack of a generated ButtonRelease event on the XTEST pointer device means the button is never released, resulting in the stuck button observed above. Solution: This patch merges the MD's lockedPtrButtons with the one of all attached slave devices on release events. Thus, as long as one attached keyboard has a lockedPtrButtons bit set, this bit is kept in the MD. Once a PointerKey button is released on all keyboards, the matching release event is emulated from the MD through the XTEST pointer device, thus also releasing the button in the DIX. Signed-off-by: Peter Hutterer --- include/xkbsrv.h | 3 +++ xkb/xkbAccessX.c | 18 +++++++++++++++++- xkb/xkbActions.c | 8 ++++++++ xkb/xkbUtils.c | 26 ++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/include/xkbsrv.h b/include/xkbsrv.h index 5847e6395..956b2249b 100644 --- a/include/xkbsrv.h +++ b/include/xkbsrv.h @@ -924,6 +924,9 @@ extern int XkbGetEffectiveGroup( XkbStatePtr /* xkbstate */, CARD8 /* keycode */); +extern void XkbMergeLockedPtrBtns( + DeviceIntPtr /* master */); + #include "xkbfile.h" #include "xkbrules.h" diff --git a/xkb/xkbAccessX.c b/xkb/xkbAccessX.c index 6d17c75b1..81f959677 100644 --- a/xkb/xkbAccessX.c +++ b/xkb/xkbAccessX.c @@ -707,8 +707,24 @@ DeviceEvent *event = &ev->device_event; changed |= XkbPointerButtonMask; } else if (event->type == ET_ButtonRelease) { - if (xkbi) + if (xkbi) { xkbi->lockedPtrButtons&= ~(1 << (event->detail.key & 0x7)); + + /* Merge this MD's lockedPtrButtons with the one of all + * attached slave devices. + * The DIX uses a merged button state for MDs, not + * releasing buttons until the last SD has released + * thenm. If we unconditionally clear the + * lockedPtrButtons bit on the MD, a PointerKeys button + * release on the SD keyboard won't generate the required fake button + * event on the XTEST pointer, thus never processing the + * button event in the DIX and the XTEST pointer's + * buttons stay down - result is a stuck button. + */ + if (IsMaster(dev)) + XkbMergeLockedPtrBtns(dev); + } + changed |= XkbPointerButtonMask; } diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c index d1149a196..b4b839558 100644 --- a/xkb/xkbActions.c +++ b/xkb/xkbActions.c @@ -634,6 +634,14 @@ _XkbFilterPointerBtn( XkbSrvInfoPtr xkbi, } xkbi->lockedPtrButtons&= ~(1<device)) + { + XkbMergeLockedPtrBtns(xkbi->device); + /* One SD still has lock set, don't post event */ + if ((xkbi->lockedPtrButtons & (1 << button)) != 0) + break; + } + /* fallthrough */ case XkbSA_PtrBtn: XkbFakeDeviceButton(xkbi->device, 0, button); diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c index 3344e5088..14dc784b8 100644 --- a/xkb/xkbUtils.c +++ b/xkb/xkbUtils.c @@ -2094,3 +2094,29 @@ XkbGetEffectiveGroup(XkbSrvInfoPtr xkbi, XkbStatePtr xkbState, CARD8 keycode) return effectiveGroup; } + +/* Merge the lockedPtrButtons from all attached SDs for the given master + * device into the MD's state. + */ +void +XkbMergeLockedPtrBtns(DeviceIntPtr master) +{ + DeviceIntPtr d = inputInfo.devices; + XkbSrvInfoPtr xkbi = NULL; + + if (!IsMaster(master)) + return; + + if (!master->key) + return; + + xkbi = master->key->xkbInfo; + xkbi->lockedPtrButtons = 0; + + for (; d; d = d->next) { + if (IsMaster(d) || GetMaster(d, MASTER_KEYBOARD) != master || !d->key) + continue; + + xkbi->lockedPtrButtons |= d->key->xkbInfo->lockedPtrButtons; + } +} From 339f62b1bfadb0ee77d67e351f4e30f5d5e9625f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 29 Jun 2010 15:24:51 +1000 Subject: [PATCH 6/7] xkb: emulate PointerKeys events only on the master device. This patch replicates the behaviour for button events. Only generate a PointerKeys motion event on the master device, not on the slave device. Fixes the current issue of PointerKey motion events generating key events as well. Signed-off-by: Peter Hutterer --- xkb/xkbActions.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c index b4b839558..49d7b3d53 100644 --- a/xkb/xkbActions.c +++ b/xkb/xkbActions.c @@ -500,9 +500,6 @@ _XkbFilterPointerMove( XkbSrvInfoPtr xkbi, int x,y; Bool accel; - if (xkbi->device == inputInfo.keyboard) - return 0; - if (filter->keycode==0) { /* initial press */ filter->keycode = keycode; filter->active = 1; @@ -1342,10 +1339,12 @@ XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y) DeviceIntPtr ptr; int gpe_flags = 0; - if (!dev->u.master) + if (IsMaster(dev)) + ptr = GetXTestDevice(GetMaster(dev, MASTER_POINTER)); + else if (!dev->u.master) ptr = dev; else - ptr = GetXTestDevice(GetMaster(dev, MASTER_POINTER)); + return; if (flags & XkbSA_MoveAbsoluteX || flags & XkbSA_MoveAbsoluteY) gpe_flags = POINTER_ABSOLUTE; From 14327858391ebe929b806efb53ad79e789361883 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 1 Jul 2010 12:44:57 +1000 Subject: [PATCH 7/7] xkb: release XTEST pointer buttons on physical releases. (#28808) If a button release event is posted for the MD pointer, post a release event through the matching XTEST device. This way, a client who posts a button press through the XTEST extension cannot inadvertedly lock the button. This behaviour is required for historical reasons, until server 1.7 the core pointer would release a button press on physical events, regardless of the XTEST state. Clients seem to rely on this behaviour, causing seemingly stuck grabs. The merged behaviour is kept for multiple keyboard PointerKey events, if two physical keyboards hold the button down as a result of PointerKey actions, the button is not released until the last keyboard releases the button. X.Org Bug 28808 Signed-off-by: Peter Hutterer --- include/xkbsrv.h | 6 ++++++ xkb/xkbAccessX.c | 23 ++++++++++------------- xkb/xkbActions.c | 4 ++-- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/include/xkbsrv.h b/include/xkbsrv.h index 956b2249b..9f1507e8e 100644 --- a/include/xkbsrv.h +++ b/include/xkbsrv.h @@ -927,6 +927,12 @@ extern int XkbGetEffectiveGroup( extern void XkbMergeLockedPtrBtns( DeviceIntPtr /* master */); +extern void XkbFakeDeviceButton( + DeviceIntPtr /* dev */, + int /* press */, + int /* button */); + + #include "xkbfile.h" #include "xkbrules.h" diff --git a/xkb/xkbAccessX.c b/xkb/xkbAccessX.c index 81f959677..10c38ca47 100644 --- a/xkb/xkbAccessX.c +++ b/xkb/xkbAccessX.c @@ -710,19 +710,16 @@ DeviceEvent *event = &ev->device_event; if (xkbi) { xkbi->lockedPtrButtons&= ~(1 << (event->detail.key & 0x7)); - /* Merge this MD's lockedPtrButtons with the one of all - * attached slave devices. - * The DIX uses a merged button state for MDs, not - * releasing buttons until the last SD has released - * thenm. If we unconditionally clear the - * lockedPtrButtons bit on the MD, a PointerKeys button - * release on the SD keyboard won't generate the required fake button - * event on the XTEST pointer, thus never processing the - * button event in the DIX and the XTEST pointer's - * buttons stay down - result is a stuck button. - */ - if (IsMaster(dev)) - XkbMergeLockedPtrBtns(dev); + if (IsMaster(dev)) + { + DeviceIntPtr source; + int rc; + rc = dixLookupDevice(&source, event->sourceid, serverClient, DixWriteAccess); + if (rc != Success) + ErrorF("[xkb] bad sourceid '%d' on button release event.\n", event->sourceid); + else if (!IsXTestDevice(source, GetMaster(dev, MASTER_POINTER))) + XkbFakeDeviceButton(dev, FALSE, event->detail.key); + } } changed |= XkbPointerButtonMask; diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c index 49d7b3d53..96d3847b9 100644 --- a/xkb/xkbActions.c +++ b/xkb/xkbActions.c @@ -45,7 +45,7 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE. DevPrivateKeyRec xkbDevicePrivateKeyRec; -static void XkbFakeDeviceButton(DeviceIntPtr dev,Bool press,int button); +void XkbFakeDeviceButton(DeviceIntPtr dev,Bool press,int button); static void XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y); void @@ -1364,7 +1364,7 @@ XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y) FreeEventList(events, GetMaximumEventsNum()); } -static void +void XkbFakeDeviceButton(DeviceIntPtr dev,Bool press,int button) { EventListPtr events;