Bug 178170 - [patch] x11-servers/xorg-server: xkb misbehaviour on keyboard layouts switch
Summary: [patch] x11-servers/xorg-server: xkb misbehaviour on keyboard layouts switch
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Eugene Grosbein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-26 06:10 UTC by Eugene Grosbein
Modified: 2017-03-16 17:22 UTC (History)
1 user (show)

See Also:


Attachments
patch updated for xorg-server-1.14.7 (3.06 KB, patch)
2015-09-16 09:04 UTC, Eugene Grosbein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Grosbein 2013-04-26 06:10:00 UTC
	The problem described here is exactly identical to
	https://bugs.freedesktop.org/show_bug.cgi?id=865

	In short, xkb switches keyboard layouts on press of switching key
	combination. This breaks application hotkeys like Ctrl-Shift-A
	when your layout switching key combo is Ctrl-Shift. xkb should
	switch layouts on key release, not press, like Windows GUI does.

Fix: 

The patch taken from mentioned freedesktop.org bug solves the problem
	by changing xkb behaviour to switch layouts on key release.

	https://bugs.freedesktop.org/attachment.cgi?id=33142

	The patch should be added to ports tree as
	ports/x11-servers/xorg-server/files/patch-xkbActions.c

--- xkb/xkbActions.c
+++ xkb/xkbActions.c
@@ -325,24 +325,83 @@ _XkbFilterLatchState(	XkbSrvInfoPtr	xkbi,
     return 1;
 }
 
+static int xkbSwitchGroupOnRelease(void)
+{
+    /* TODO: user configuring */
+    return TRUE;
+}
+
+static void xkbUpdateLockedGroup(XkbSrvInfoPtr xkbi, XkbAction* pAction)
+{
+    XkbGroupAction ga = pAction->group;
+    if (ga.flags&XkbSA_GroupAbsolute)
+	xkbi->state.locked_group= XkbSAGroup(&ga);
+    else xkbi->state.locked_group+= XkbSAGroup(&ga);
+}
+
+static XkbFilterPtr _XkbNextFreeFilter(XkbSrvInfoPtr xkbi);
+
 static int
-_XkbFilterLockState(	XkbSrvInfoPtr	xkbi,
+_XkbFilterLockGroup(	XkbSrvInfoPtr	xkbi,
 			XkbFilterPtr	filter,
 			unsigned	keycode,
 			XkbAction *	pAction)
 {
-    if (pAction&&(pAction->type==XkbSA_LockGroup)) {
-	if (pAction->group.flags&XkbSA_GroupAbsolute)
-	     xkbi->state.locked_group= XkbSAGroup(&pAction->group);
-	else xkbi->state.locked_group+= XkbSAGroup(&pAction->group);
-	return 1;
+    int sendEvent = 1;
+
+    if (!xkbSwitchGroupOnRelease()) {
+	xkbUpdateLockedGroup(xkbi, pAction);
+	return sendEvent;
+    }
+    
+    /* Delay switch till button release */
+    if (filter->keycode==0) {		/* initial press */
+	filter->keycode = keycode;
+	filter->active = 1;
+	filter->filterOthers = 0; /* for what? */
+	filter->filter = _XkbFilterLockGroup;
+
+	/* filter->priv = 0; */
+	filter->upAction = *pAction;
+
+	/* Ok, now we need to simulate the action which would go if this action didn't block it.
+	   XkbSA_SetMods is the one: it is to set modifier' flag up. */
+	{
+	    XkbStateRec	fake_state = xkbi->state;
+	    XkbAction act;
+
+	    fake_state.mods = 0;
+	    act = XkbGetKeyAction(xkbi, &fake_state, keycode);
+
+	    /* KLUDGE: XkbSA_SetMods only? */
+	    if (act.type == XkbSA_SetMods) { 
+		XkbFilterPtr filter = _XkbNextFreeFilter(xkbi);
+		sendEvent = _XkbFilterSetState(xkbi,filter,keycode,&act);
+	    }
+	}
     }
+    else {
+  	/* do nothing if some button else is pressed */
+	if (!pAction)
+	    xkbUpdateLockedGroup(xkbi, &filter->upAction);
+	filter->active = 0;
+    }
+
+    return sendEvent;
+}
+
+static int
+_XkbFilterLockMods(	XkbSrvInfoPtr	xkbi,
+			XkbFilterPtr	filter,
+			unsigned	keycode,
+			XkbAction *	pAction)
+{
     if (filter->keycode==0) {		/* initial press */
 	filter->keycode = keycode;
 	filter->active = 1;
 	filter->filterOthers = 0;
 	filter->priv = 0;
-	filter->filter = _XkbFilterLockState;
+	filter->filter = _XkbFilterLockMods;
 	filter->upAction = *pAction;
 	xkbi->state.locked_mods^= pAction->mods.mask;
 	xkbi->setMods = pAction->mods.mask;
@@ -1104,9 +1163,12 @@ xkbDeviceInfoPtr xkbPrivPtr = XKBDEVICEINFO(dev);
 		    sendEvent=_XkbFilterLatchState(xkbi,filter,key,&act);
 		    break;
 		case XkbSA_LockMods:
+		    filter = _XkbNextFreeFilter(xkbi);
+		    sendEvent=_XkbFilterLockMods(xkbi,filter,key,&act);
+		    break;
 		case XkbSA_LockGroup:
 		    filter = _XkbNextFreeFilter(xkbi);
-		    sendEvent=_XkbFilterLockState(xkbi,filter,key,&act);
+		    sendEvent=_XkbFilterLockGroup(xkbi,filter,key,&act);
 		    break;
 		case XkbSA_ISOLock:
 		    filter = _XkbNextFreeFilter(xkbi);
How-To-Repeat: 
	The section in xorg.conf:

Section "InputDevice"
	Identifier  "Keyboard0"
	Driver      "kbd"
	Option      "XkbRules" "xorg"
	Option      "XkbModel" "pc105"
	Option      "XkbLayout" "us,ru"
	Option      "XkbVariant" "winkeys"
	Option      "XkbOptions" "grp:ctrl_shift_toggle"
EndSection

	For example, you cannot use Ctrl-Shift-A in Firefox with such config.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2013-04-26 23:44:48 UTC
Responsible Changed
From-To: freebsd-ports-bugs->freebsd-x11

Fix synopsis and assign.
Comment 2 Niclas Zeising freebsd_committer 2013-04-27 08:02:15 UTC
As we're in the middle of updating all xorg related ports, including
xorg-server, please test the CFT that was sent out earlier, and see if
the problem still exists.  Which version of xorg-server is the patch
against?
Regards!
-- 
Niclas
Comment 3 Eugene Grosbein 2013-04-28 13:36:29 UTC
On 27.04.2013 14:02, Niclas Zeising wrote:
> As we're in the middle of updating all xorg related ports, including
> xorg-server, please test the CFT that was sent out earlier, and see if
> the problem still exists.  Which version of xorg-server is the patch
> against?
> Regards!
> 

Sorry, I forgot to mention that patch applies cleanly to xorg-server-1.7.7_6,1
despite its age, the patch applied to xorg-server-1.6.5 too.

There is report in mentioned freedesktop bug discussion the problem here in xorg-server 1.14.
It seems, not nuch has changed in this xkb code.

I've read CFT letter. I'm legacy NVidia hardware user, it seems I should not touch newer X.org?

Eugene Grosbein
Comment 4 Niclas Zeising freebsd_committer 2013-06-27 13:05:22 UTC
It seems like this is still not resolved upstream.  I'm hesitant to
apply this patch, since it is not accepted upstream, and might have
unforseen consequences.  Feel free to poke upstream and see why it's not
accepted.
Are any other distributions (BSD/Linux) using this patch?
Regards!
-- 
Niclas Zeising
Comment 5 Niclas Zeising freebsd_committer 2013-06-27 15:47:50 UTC
State Changed
From-To: open->analyzed

Analyzed
Comment 6 Eugene Grosbein 2013-06-28 08:28:20 UTC
On 27.06.2013 19:05, Niclas Zeising wrote:
> It seems like this is still not resolved upstream.  I'm hesitant to
> apply this patch, since it is not accepted upstream, and might have
> unforseen consequences.  Feel free to poke upstream and see why it's not
> accepted.
> Are any other distributions (BSD/Linux) using this patch?
> Regards!

None I'm aware of. It seems, the problem and the fix do not bother
English-only people like X.org or Linux developers.

You could add this patch to EXTRA_PATCHES and create new port option,
disabled by default.

Eugene Grosbein
Comment 7 Eugene Grosbein 2015-09-16 09:04:01 UTC
Created attachment 161113 [details]
patch updated for xorg-server-1.14.7

The problem persists for xorg-server-1.14.7 too.
I updated patch and it helps.
Comment 8 Eugene Grosbein freebsd_committer 2017-03-11 19:08:03 UTC
My PR.
Comment 9 Matthew Rezny freebsd_committer 2017-03-16 17:22:53 UTC
(In reply to Eugene Grosbein from comment #8)

I do think it best to make this an optional patch in case there are unexpected effects. I use three keyboard layouts and switch with key combination that conflicts with nothing else, Shift + Caps Lock, so the current behavior is not bothersome, but I can see how it would be if you want to overload key combos.

The last patch provided here does not cleanly apply to the upcoming X server. Please update the patch for 1.19 so it can be included.