[PATCH weston v2 3/3] compositor-x11: remove manual mouse button grab/ungrab

[Benoit Gschwind]

Hello Daniel,

You are right, my explanation is too general. For the story, I looked at
x11-backend because during my use of this backend I meet several
unexpected behavior of my cursor, but at the time I didn't figured out
the real issue. Then I see how the grab is handled in x11-backend, I was
worried only based on my experience with Xserver grab, thus I proposed
the change of this patch that is safer. Now I know why
button_event->time does not solve anything, it's because the issue is
not related to this button_event->time, the issue is related to button
states. The current code (i.e. the code without the patch) stop the grab
on every button release events which is wrong because release events can
be sent if any button is release thus just try following step trigger a
bug :

1. start weston with x11-backend and start a weston-terminal.
2. left click on weston-terminal title to start a grab.
3. move the window around, every thing is ok for now
4. hold left click and click the right button of the mouse and release
it. The bug start here, you still holding left click, but when you
release the right button a release event is send and the x11-backend
ungrab the mouse.
5. move the mouse around, now your cursor is not grabbed anymore but the
backend still think he grab the cursor. The cursor is not confined to
the window anymore, clicking in other window outside the backend window
trigger unexpected behavior within the x11-backend. Just try several
senario. I can get case where my mouse event are just ignored in the
x11-backend. I the point of view of the user the backend is frozen.

I will update the patch log.

Best regard

On 23/06/2016 09:55, Daniel Stone wrote:
> Hi,
> 
> On 19 May 2016 at 05:32, Benoit Gschwind <gschwind at gnu-log.net> wrote:
>> In current compositor-x11, the mouse buttons are grabbed and ungrabbed
>> manually that may produce weird cases like starting a grab while the
>> buttons are already released, due to asynchronous X11 events dispatching.
>>
>> The patch avoid the issue by using the better passive button grab, that
>> automatically grab buttons as soon as they occur. The passive grab
>> include an automatic ungrab when all mouse button are released.
>>
>> This probably fix some mysterious bugs.
> 
> Hm, have you observed this in practice? Passing button_event->time,
> rather than CurrentTime, should ensure that the grab does not take
> effect if there has been another button press, since I believe that
> will subsequently update the last grab time in the server. Even if it
> did, we would ungrab the pointer as soon as we receive the button
> release - which we are guaranteed to do thanks to implicit grabs.
> 
> I think it's a reasonable and good change to have, but am not
> convinced by the reasoning for it.
> 
> Cheers,
> Daniel
>