[PATCH] Xi: Fix master button update when slave buttons are mapped. #24887

[Peter Hutterer]

On Sun, Nov 28, 2010 at 04:15:51PM -0500, Eoghan Sherry wrote:
> It is currently assumed that an event button delieved to a master device
> corresponds to the slave button states. However, the event button is a
> logical (mapped) slave button and slave button states correspond to
> physical (unmapped) slave buttons. This leads to incorrect update of the
> master button state and incorrect events devlivered to clients. Fix the
> situation by taking the slave button map into account when querying a
> slave button state.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=24887
> 
> Signed-off-by: Eoghan Sherry <ejsherry at gmail.com>
> ---
> 
> This bug seems to have been introduced a couple of years ago by commit
> cfcb3da7.  It probably affects xserver 1.5.0 onward.  The bug report
> describes a method of reproduction.  I came across it by using mouse
> chords (acme user here) with the mouse mapped for my left hand.
> 
> A couple of notes about the proposed fix.  The reverse map of slave
> buttons (logical to physical) could be precomputed, not sure if it's
> worthwhile. 

not really. the effort of storage, making sure it is up-to-date, etc. is
bigger than the loop run on button releases.

> Also, maybe the button state of a device could be changed
> to correspond to logical buttons, not sure of all the consequences or
> if it would even work.
> 
> Finally, this is my first contact with X.org; call me out on things I
> get wrong with the process.
> 
> Eoghan
> 
>  Xi/exevents.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Xi/exevents.c b/Xi/exevents.c
> index d57265e..8615fd4 100644
> --- a/Xi/exevents.c
> +++ b/Xi/exevents.c
> @@ -870,8 +870,10 @@ UpdateDeviceState(DeviceIntPtr device, DeviceEvent* event)
>                      continue;
>                  if (!sd->button)
>                      continue;
> -                if (button_is_down(sd, key, BUTTON_PROCESSED))
> -                    return DONT_PROCESS;
> +                for (i = 1; i <= sd->button->numButtons; i++)
> +                    if (sd->button->map[i] == key &&
> +                        button_is_down(sd, i, BUTTON_PROCESSED))
> +                        return DONT_PROCESS;
>              }
>          }
>          set_button_up(device, key, BUTTON_PROCESSED);
> -- 
> 1.7.0.4

thanks, merged.

Cheers,
  Peter