xkb: don't setMods locking modifiers (#27903)

Submitted by Peter Hutterer on July 28, 2011, 11:36 a.m.

Details

Message ID 20110728043607.GA13116@barra.bne.redhat.com
State Deferred, archived
Headers show

Not browsing as part of any series.

Commit Message

Peter Hutterer July 28, 2011, 11:36 a.m.
Locking modifiers (e.g. CapsLock) currently lock the modifier on the first
press and release on the second press. The modifier locking in more details
however works like this:
press   -> modifier locked,     event state has modifier up
release -> modifier locked,     event state has modifier down
press   -> modifier released,   event state has modifier down
release -> modifier released,   event state has modifier down

The second press changes the locked_modifiers, but it also sets the modifier
bit for the current press, effectively still setting the modifier from the
client's perspective. The modifier is not effectively unlocked until the
second _release_ event.

Change this behaviour so that the second key _press_ event releases the
modifier, changing to a sequence of:

press   -> modifier locked,     event state has modifier up
release -> modifier locked,     event state has modifier down
press   -> modifier released,   event state has modifier down
release -> modifier released,   event state has modifier up

As a result, if the modifier is pressed for the second time and held down,
following events have the modifier bits unset.

X.Org Bug 27903 <http://bugs.freedesktop.org/show_bug.cgi?id=27903>

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
I'm not sure there are any other side effects visible from this but the most
obvious one is that if you press, release, then press CapsLock, you'll now
end up with lowercase where it used to be uppercase. 

Thus, if you're a fast typer, the sequence "CapsLock F CapsLock oobar" goes
from "FOobar" to "Foobar" (see bug 27903). I'm not sure there's a use-case
for the current behaviour, so changing this doesn't seem to impact badly.

 xkb/xkbActions.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

Patch hide | download patch | download mbox

diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index 4b5405a..9e1d39b 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -348,7 +348,11 @@  _XkbFilterLockState(	XkbSrvInfoPtr	xkbi,
 	filter->filter = _XkbFilterLockState;
 	filter->upAction = *pAction;
 	xkbi->state.locked_mods^= pAction->mods.mask;
-	xkbi->setMods = pAction->mods.mask;
+	/* Don't actually change setMods here. We're locking modifiers, so
+	 * the combined results of base + locked is the same regardless. By
+	 * not setting them, we get to release locked modifiers on the
+	 * second key _press_, not release (Bug 27903)
+	 */
     }
     else if (filter->keycode==keycode) {
 	filter->active = 0;

Comments

Hi,

On 28 July 2011 05:36, Peter Hutterer <peter.hutterer@who-t.net> wrote:
> @@ -348,7 +348,11 @@ _XkbFilterLockState(       XkbSrvInfoPtr   xkbi,
>        filter->filter = _XkbFilterLockState;
>        filter->upAction = *pAction;
>        xkbi->state.locked_mods^= pAction->mods.mask;
> -       xkbi->setMods = pAction->mods.mask;
> +       /* Don't actually change setMods here. We're locking modifiers, so
> +        * the combined results of base + locked is the same regardless. By
> +        * not setting them, we get to release locked modifiers on the
> +        * second key _press_, not release (Bug 27903)
> +        */

This mostly seems OK to me, but breaks the LockNoLock state; I guess
you'd want to directly change base_mods in the other branch.  With
that fixed:
Acked-by: Daniel Stone <daniel@fooishbar.org>

Cheers,
Daniel
On Sun, Apr 22, 2012 at 05:25:35PM +0100, Daniel Stone wrote:
> Hi,
> 
> On 28 July 2011 05:36, Peter Hutterer <peter.hutterer@who-t.net> wrote:
> > @@ -348,7 +348,11 @@ _XkbFilterLockState(       XkbSrvInfoPtr   xkbi,
> >        filter->filter = _XkbFilterLockState;
> >        filter->upAction = *pAction;
> >        xkbi->state.locked_mods^= pAction->mods.mask;
> > -       xkbi->setMods = pAction->mods.mask;
> > +       /* Don't actually change setMods here. We're locking modifiers, so
> > +        * the combined results of base + locked is the same regardless. By
> > +        * not setting them, we get to release locked modifiers on the
> > +        * second key _press_, not release (Bug 27903)
> > +        */
> 
> This mostly seems OK to me, but breaks the LockNoLock state; I guess
> you'd want to directly change base_mods in the other branch.  With
> that fixed:
> Acked-by: Daniel Stone <daniel@fooishbar.org>

fwiw, obsoleted by Andreas' e3f6a76dd480717eae4b17ad8e2ff707de2ffe4c.
  xkb: Support noLock and noUnlock flags for LockMods

Cheers,
  Peter