xkb: Fixes to LatchMods/LatchGroup

The main problem this patch addresses is that if a latch is put on
multi-level key with a Latch/Lock/Set, it is possible that after all
keys are released, still base modifiers are set, which typically will
make the keyboard unusable.  To see how it happens (without the patch),
assume that key AltGr sets Mod5 when pressed by itself, and latches Mod3
when pressed together with Shift.  Now press Shift, then AltGr and
release both keys in reverse order.  Mod3 is now latched, and the
LatchMods filter remains active as the second filter.  Now press AltGr;
Mod5 base modifier gets set, and the SetMods filter will become active
as the first filter.  Release AltGr: First, the SetMods filter will set
clearMods to Mod5, then the LatchMods filter will overwrite clearMods
with Mod3.  Result: the Mod5 base modifier will remain set.  This
example becomes practically relevant for the revised German standard
layout (DIN 2137-1:2012-06).

Other changes implement the latch behaviour more accurately according to
the specification.  For example, releasing a modifier latching key can
at the same time clear a locked modifier, promote another modifier that
is latched to locked, and latch a third modifier.  Overall, what the
code does should be straightforward to compare what the XKB protocol
specification demands, see the table in section 6.3.

Finally, releasing a key no longer cancels a latch that has not become
pending yet.  In my opinion, the specification is not clear; it speaks
of "operating" a key, which the patch effectivly interprets as "press"
rather than "press or release".  From my experience, using the latter
interpretation makes latches on higher levels practically unusable.  In
the example given above, one would have to release AltGr always before
Shift to get the Mod3-Latch.  The practical relevance of latches on
higher levels is once more given by the revised German standard layout.

Signed-off-by: Andreas Wettstein <wettstein509@solnet.ch>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

1 files changed, 80 insertions, 69 deletions

diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index 416de925d..e32005cf6 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -222,7 +222,6 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi,
 
 #define	LATCH_KEY_DOWN	1
 #define	LATCH_PENDING	2
-#define	NO_LATCH	3
 
 static int
 _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
@@ -230,6 +229,7 @@ _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
 {
 
     if (filter->keycode == 0) { /* initial press */
+        AccessXCancelRepeatKey(xkbi,keycode);
         filter->keycode = keycode;
         filter->active = 1;
         filter->filterOthers = 1;
@@ -250,91 +250,102 @@ _XkbFilterLatchState(XkbSrvInfoPtr xkbi,
     else if (pAction && (filter->priv == LATCH_PENDING)) {
         if (((1 << pAction->type) & XkbSA_BreakLatch) != 0) {
             filter->active = 0;
-            if (filter->upAction.type == XkbSA_LatchMods)
-                xkbi->state.latched_mods &= ~filter->upAction.mods.mask;
-            else
-                xkbi->state.latched_group -=
-                    XkbSAGroup(&filter->upAction.group);
-        }
-        else if ((pAction->type == filter->upAction.type) &&
-                 (pAction->mods.flags == filter->upAction.mods.flags) &&
-                 (pAction->mods.mask == filter->upAction.mods.mask)) {
-            if (filter->upAction.mods.flags & XkbSA_LatchToLock) {
-                XkbControlsPtr ctrls = xkbi->desc->ctrls;
-
-                if (filter->upAction.type == XkbSA_LatchMods)
-                    pAction->mods.type = XkbSA_LockMods;
-                else
-                    pAction->group.type = XkbSA_LockGroup;
-                if (XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask) &&
-                    (ctrls->enabled_ctrls & XkbStickyKeysMask)) {
-                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
-                                      XkbStickyKeysMask);
-                }
-            }
-            else {
-                if (filter->upAction.type == XkbSA_LatchMods)
-                    pAction->mods.type = XkbSA_SetMods;
-                else
-                    pAction->group.type = XkbSA_SetGroup;
-            }
-            if (filter->upAction.type == XkbSA_LatchMods)
-                xkbi->state.latched_mods &= ~filter->upAction.mods.mask;
-            else
-                xkbi->state.latched_group -=
-                    XkbSAGroup(&filter->upAction.group);
-            filter->active = 0;
+            /* If one latch is broken, all latches are broken, so it's no use
+               to find out which particular latch this filter tracks. */
+            xkbi->state.latched_mods = 0;
+            xkbi->state.latched_group = 0;
         }
     }
-    else if (filter->keycode == keycode) {      /* release */
+    else if (filter->keycode == keycode && filter->priv != LATCH_PENDING){
+        /* The test above for LATCH_PENDING skips subsequent releases of the
+           key after it has been released first time and the latch became
+           pending. */
         XkbControlsPtr ctrls = xkbi->desc->ctrls;
-        int needBeep;
-        int beepType = _BEEP_NONE;
+        int needBeep = ((ctrls->enabled_ctrls & XkbStickyKeysMask) &&
+                        XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask));
 
-        needBeep = ((ctrls->enabled_ctrls & XkbStickyKeysMask) &&
-                    XkbAX_NeedFeedback(ctrls, XkbAX_StickyKeysFBMask));
         if (filter->upAction.type == XkbSA_LatchMods) {
-            xkbi->clearMods = filter->upAction.mods.mask;
-            if ((filter->upAction.mods.flags & XkbSA_ClearLocks) &&
-                (xkbi->clearMods & xkbi->state.locked_mods) ==
-                xkbi->clearMods) {
-                xkbi->state.locked_mods &= ~xkbi->clearMods;
-                filter->priv = NO_LATCH;
-                beepType = _BEEP_STICKY_UNLOCK;
+            unsigned char mask = filter->upAction.mods.mask;
+            unsigned char common;
+
+            xkbi->clearMods = mask;
+
+            /* ClearLocks */
+            common = mask & xkbi->state.locked_mods;
+            if ((filter->upAction.mods.flags & XkbSA_ClearLocks) && common) {
+                mask &= ~common;
+                xkbi->state.locked_mods &= ~common;
+                if (needBeep)
+                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_UNLOCK,
+                                      XkbStickyKeysMask);
+            }
+            /* LatchToLock */
+            common = mask & xkbi->state.latched_mods;
+            if ((filter->upAction.mods.flags & XkbSA_LatchToLock) && common) {
+                unsigned char newlocked;
+
+                mask &= ~common;
+                newlocked = common & ~xkbi->state.locked_mods;
+                if(newlocked){
+                    xkbi->state.locked_mods |= newlocked;
+                    if (needBeep)
+                        XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
+                                          XkbStickyKeysMask);
+
+                }
+                xkbi->state.latched_mods &= ~common;
+            }
+            /* Latch remaining modifiers, if any. */
+            if (mask) {
+                xkbi->state.latched_mods |= mask;
+                filter->priv = LATCH_PENDING;
+                if (needBeep)
+                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LATCH,
+                                      XkbStickyKeysMask);
             }
         }
         else {
             xkbi->groupChange = -XkbSAGroup(&filter->upAction.group);
+            /* ClearLocks */
             if ((filter->upAction.group.flags & XkbSA_ClearLocks) &&
                 (xkbi->state.locked_group)) {
                 xkbi->state.locked_group = 0;
-                filter->priv = NO_LATCH;
-                beepType = _BEEP_STICKY_UNLOCK;
+                if (needBeep)
+                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_UNLOCK,
+                                      XkbStickyKeysMask);
             }
-        }
-        if (filter->priv == NO_LATCH) {
-            filter->active = 0;
-        }
-        else {
-            filter->priv = LATCH_PENDING;
-            if (filter->upAction.type == XkbSA_LatchMods) {
-                xkbi->state.latched_mods |= filter->upAction.mods.mask;
-                needBeep = xkbi->state.latched_mods ? needBeep : 0;
-                xkbi->state.latched_mods |= filter->upAction.mods.mask;
+            /* LatchToLock */
+            else if ((filter->upAction.group.flags & XkbSA_LatchToLock)
+                     && (xkbi->state.latched_group)) {
+                xkbi->state.locked_group  += XkbSAGroup(&filter->upAction.group);
+                xkbi->state.latched_group -= XkbSAGroup(&filter->upAction.group);
+                if(XkbSAGroup(&filter->upAction.group) && needBeep)
+                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LOCK,
+                                      XkbStickyKeysMask);
             }
-            else {
-                xkbi->state.latched_group +=
-                    XkbSAGroup(&filter->upAction.group);
+            /* Latch group */
+            else if(XkbSAGroup(&filter->upAction.group)){
+                xkbi->state.latched_group += XkbSAGroup(&filter->upAction.group);
+                filter->priv = LATCH_PENDING;
+                if (needBeep)
+                    XkbDDXAccessXBeep(xkbi->device, _BEEP_STICKY_LATCH,
+                                      XkbStickyKeysMask);
             }
-            if (needBeep && (beepType == _BEEP_NONE))
-                beepType = _BEEP_STICKY_LATCH;
         }
-        if (needBeep && (beepType != _BEEP_NONE))
-            XkbDDXAccessXBeep(xkbi->device, beepType, XkbStickyKeysMask);
+
+        if (filter->priv != LATCH_PENDING)
+            filter->active = 0;
     }
-    else if (filter->priv == LATCH_KEY_DOWN) {
-        filter->priv = NO_LATCH;
-        filter->filterOthers = 0;
+    else if (pAction && (filter->priv == LATCH_KEY_DOWN)) {
+        /* Latch was broken before it became pending: degrade to a
+           SetMods/SetGroup. */
+        if (filter->upAction.type == XkbSA_LatchMods)
+            filter->upAction.type = XkbSA_SetMods;
+        else
+            filter->upAction.type = XkbSA_SetGroup;
+        filter->filter = _XkbFilterSetState;
+        filter->priv = 0;
+        return filter->filter(xkbi, filter, keycode, pAction);
     }
     return 1;
 }