[PATCH try2] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Peter Hutterer]

Hi Andrew,

sorry about the long delay here.

On Fri, May 15, 2015 at 03:10:13PM -0500, Andrew Eikum wrote:
> While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
> currently never actually invoked by the xserver.
> 
> This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
> processing was moved into XKB, the hook was forgotten and silently
> dropped. The code calling this hook was removed by 7af53799c (4 Jan
> 2009), but it was probably already unused before that.
> 
> This patch re-adds support for this hook. The "count" hook parameter is
> unused.
> 
> Signed-off-by: Andrew Eikum <aeikum at codeweavers.com>
> ---
> 
> Change from previous patch: Daniel S on IRC suggested placing the hook
> into ProcessDeviceEvent, as that's where the X server does most of the
> actual event prep and handling before dispatch to clients.
> 
> I noticed while testing this that my XACE plugin got multiple calls for
> a single physical keypress. One came from the physical keyboard device,
> and another from the Virtual core keyboard. I thought about doing some
> sort of filtering to prevent duplicate events, but I think it's better
> to ask the XACE plugin to do its own discrimination based on the device.

yeah, that's intentional, the key events feed through the real device into
the VCK and then are sent to the caller. this allows clients (and XACE) to
restrict access to specific devices or the keyboard focus in general.

> Pending approval, updates to the XACE spec documentation for the "count"
> parameter, and to warn about duplicate events, will follow.

that documentation is wrong (well, it says it's unsure so I guess that's ok
then :)

many moons ago events from input devices would be stored in an xEvent struct
and pushed to the internal event queue (events are generated during SIGIO).
xEvents have size limits and are restricted to two axes. XI 1.x works around
that by having one DeviceMotion (etc.) event followed by a couple of
DeviceValuator events (up to 6). so for 'core devices' count was 1 and the
event was a core X event, for extension devices count was 1-7.

two things happened:
* the distinction between core/extension devices went dodo, all devices can
  send extension events now
* we switched to InternalEvents for the event queue which have no size
  restriction, now we convert to the xEvent when we know we have to send one
  of those

either way, EventToCore and EventToXI take care of this appropriately.

> ---
>  Xi/exevents.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Xi/exevents.c b/Xi/exevents.c
> index 1c586d0..859c29f 100644
> --- a/Xi/exevents.c
> +++ b/Xi/exevents.c
> @@ -1730,6 +1730,18 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr device)
>          break;
>      }
>  
> +    /* send KeyPress and KeyRelease events to XACE plugins */
> +    if (XaceHooks[XACE_KEY_AVAIL] &&

I don't think accessing the hooks directly is a good idea. I understand this
optimizes things but it'd be better to have a XaceHookIsSet() helper for
this.

> +            (event->type == ET_KeyPress || event->type == ET_KeyRelease)) {
> +        xEvent *core;
> +        int count;
> +
> +        if (EventToCore(ev, &core, &count) == Success) {
> +            XaceHook(XACE_KEY_AVAIL, core, device, 0);
> +            free(core);
> +        }

to be strict in line with the old code you'd also have to do EventToXI here.
Not that I think anyone is using this (given that it took 5 years for the
removal to be noticed :)

other than that, I think this is the right place to put it.

Cheers,
   Peter


> +    }
> +
>      if (DeviceEventCallback && !syncEvents.playingEvents) {
>          DeviceEventInfoRec eventinfo;
>          SpritePtr pSprite = device->spriteInfo->sprite;
> -- 
> 2.4.0