[PATCH 2/2] DIX/Xi: Don't grab device buttons if no grab is registered

[Peter Hutterer]

On Fri, Aug 16, 2013 at 01:56:07PM +0200, Egbert Eich wrote:
> On Fri, Aug 16, 2013 at 03:37:42PM +1000, Peter Hutterer wrote:
> > On Thu, Aug 15, 2013 at 03:49:21PM +0200, Egbert Eich wrote:
> > > Core events implicietely grab buttons on button press events, for
> > 
> > typo
> > 
> > > Xi (ie. device) button press events this is not specified.
> > 
> > it is specified, see XIproto.txt:
> >   For DeviceButtonPress events, the client may specify whether or
> >   not an implicit passive grab should be done when the button is
> >   pressed. If the client wants to guarantee that it will receive
> >   a DeviceButtonRelease event for each DeviceButtonPress event it
> >   receives, it should specify the DeviceButtonPressGrab event
> >   class as well as the DeviceButtonPress event class. This
> >   restricts the client in that only one client at a time may
> >   request DeviceButtonPress events from the same device and
> >   window if any client specifies this class.
> > 
> > I read this as default: no implicit grab, DeviceButtonPressGrab: always
> > implicit grab.
> 
> Exactly. The current behavior is to do an implicit grab, though.
> 
> > 
> > > The test clause we are removing however applied the core event
> > > behavior also to Xi events causing button release events being
> > > delivered only to a single client even if no client explicitely
> > > belongs to the DeviceButtonPressGrabClass.
> > > Note: while this behavior is reasonable there is nothing in the
> > > Xi specs that guarantees the delivery of a ButtonRelease event
> > > for every ButtonPress event if the client hasn't specifically
> > > registered for the DeviceButtonPressGrab class.
> > > In other words this patch removes the 'implicite dragging' from
> > > Xi extension button events.
> > 
> > This doesn't take the DeviceButtonPressGrab into account then.
> > how about this diff?
> > 
> > diff --git a/dix/events.c b/dix/events.c
> > index c3a0ae7..10351e3 100644
> > --- a/dix/events.c
> > +++ b/dix/events.c
> > @@ -1990,6 +1990,8 @@ ActivateImplicitGrab(DeviceIntPtr dev, ClientPtr client, WindowPtr win,
> > 
> >      if (type == ButtonPress)
> >          grabtype = CORE;
> > +    else if (type == DeviceButtonPress && (deliveryMask & DeviceButtonGrabMask))
> > +        grabtype = XI;
> 
> It does, but shouldn't this situation be taken care of by calling 
> CheckDeviceGrabAndHintWindow() a few lines down in DeliverEventsToWindow()?
> If we do this the code path in CheckDeviceGrabAndHintWindow()
>    ...
>    else if ((type == DeviceButtonPress) && (!grab) &&
>              (deliveryMask & DeviceButtonGrabMask)) {
>         GrabPtr tempGrab;
>    ...
>    }
> 
> is probably never being called.

oh, right. convoluted code...

> 
> >      else if ((type = xi2_get_type(event)) == XI_ButtonPress)
> >          grabtype = XI2;
> >      else
> > 
> > what I do wonder though is if there are clients that rely on the current
> > behaviour. did you have something break that triggered this patchset?
> 
> A customer had the issue that he wanted to receive Xi events on the same
> window with two clients simultaniously. He claimed that this was working
> with Xserver 1.5.2 which we shipped with SLE-11 originally but 'broke' 
> with the first SP when he got Xserver 1.6.5.

when was this problem being reported? wikipedia tells me SP1 was released in
June 2010. either way, if that broke in 1.6 it means we've now shipped 8
versions of X with the broken behaviour and restoring it could well break
some other application. rock, hard place, frying pan, etc.

Cheers,
   Peter