On Thu, Jan 13, 2011 at 01:45:02PM +1000, Peter Hutterer wrote:
> enabled_ctrls_changes is nowhere near the usual event or config paths. So
> this condition always evaluated to false and the memcpy would thus never
> been hit. As a result, any modification to the XKB struct during
> XkbUpdateDescActions was not reflected in the kbdfeed ctrls.
> The flag that is set by XkbUpdateDescActions() if ctrls were changed are in
> enabled_ctrls.
>
> This mainly affected keyboard repeat control as XKB uses the kbdfeed ctrls,
> not XKB's per_key_repeats, to determine if a key needs to be repeated. Thus,
> adding a "repeat= False" to the XKB map of any action did not have any
> effect.
>
> Test case:
> assign Mode_switch to any key that by default repeats, e.g. the menu key.
>
> key <COMP> { [ Mode_switch ] };
>
> Then modify the Mode_switch action to not repeat the key.
>
> interpret Mode_switch+AnyOfOrNone(all) {
> virtualModifier= AltGr;
> useModMapMods=level1;
> action= SetGroup(group=+1);
> // Add this line
> repeat= False;
> };
>
> Though the flags are correctly reflected in the description loaded in the
> server, the change is not handed back to the kbdfeed struct and XKB will
> trigger softrepeats of this key.
>
> This patch also adds two explanatory comments and an extra check, as this
> path may be hit before the CtrlProc for the kbdfeed struct is set.
>
> Red Hat Bug 537708 <https://bugzilla.redhat.com/show_bug.cgi?id=537708>
>
> Also fixes broken auto-repeat of the backspace key in the colemak layout
> (mapped to CapsLock).
>
> X.Org Bug 16318 <http://bugs.freedesktop.org/show_bug.cgi?id=16318>
>
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> ---
> xkb/xkbUtils.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
> index 14dc784..23fe57e 100644
> --- a/xkb/xkbUtils.c
> +++ b/xkb/xkbUtils.c
> @@ -342,15 +342,18 @@ CARD8 * repeat;
> xkb= xkbi->desc;
> repeat= xkb->ctrls->per_key_repeat;
>
> + /* before letting XKB do any changes, copy the current core values */
> if (pXDev->kbdfeed)
> memcpy(repeat,pXDev->kbdfeed->ctrl.autoRepeats,XkbPerKeyBitArraySize);
>
> XkbUpdateDescActions(xkb,first,num,changes);
>
> if ((pXDev->kbdfeed)&&
> - (changes->ctrls.enabled_ctrls_changes&XkbPerKeyRepeatMask)) {
> + (changes->ctrls.changed_ctrls&XkbPerKeyRepeatMask)) {
> + /* now copy the modified changes back to core */
> memcpy(pXDev->kbdfeed->ctrl.autoRepeats,repeat,
> XkbPerKeyBitArraySize);
> - (*pXDev->kbdfeed->CtrlProc)(pXDev, &pXDev->kbdfeed->ctrl);
> + if (pXDev->kbdfeed->CtrlProc)
> + (*pXDev->kbdfeed->CtrlProc)(pXDev, &pXDev->kbdfeed->ctrl);
> }
> return;
> }
> --
> 1.7.3.4
>
The continued condition could be indented an additional level.
Tested-by: Dirk Wallenstein <hals...@t-online.de>
Reviewed-by: Dirk Wallenstein <hals...@t-online.de>
--
Greetings,
Dirk
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
