Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug where the keyboard layout was changed when generating a key event #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

janekfleper
Copy link

@janekfleper janekfleper commented Feb 24, 2019

When an event is generated by xcape, the group/layout of the keyboard was always set to 0 (the default group, state 0x0??? in xev -event keyboard).

Example showing the bug

$ ./xcape -d -e "Control_L=#65"
Starting with the keyboard layout 0x2000 (group 1) I press the "j" key, then tap the Control_L key to trigger xcape and then press the "j" key again.

before, group = 1
Intercepted key event 2, key code 44
after, group = 1
jbefore, group = 1 // pressing "j"
Intercepted key event 3, key code 44
after, group = 1
before, group = 1
Intercepted key event 2, key code 66
Key pressed!
after, group = 1
before, group = 1 // before the generated event the group is still correct
Intercepted key event 3, key code 66
Key released!
Generating space!
after, group = 0 // wrong group, corresponds to keyboard state 0x0000 (DVORAK)
before, group = 0
Ignoring generated event.
before, group = 0
Ignoring generated event.
before, group = 0 // generated space at the beginning of this line
Intercepted key event 2, key code 44
after, group = 0
hbefore, group = 0 // pressing "j" will now give an "h" (DVORAK)
Intercepted key event 3, key code 44
after, group = 0

"before" corresponds to the group at the beginning of intercept() and "after" corresponds to the group at the end of intercept() just before the "exit" mark.

Example showing a change of the keyboard layout

$ ./xcape -d -e "Control_L=#65"
My keyboard configuration file: /etc/X11/xorg.conf.d/00-keyboard.conf

Section "InputClass"
        Identifier "system-keyboard"
        MatchIsKeyboard "on"
        Option "XkbLayout" "us,us,de"
        Option "XkbModel" "pc105"
        Option "XkbVariant" "dvp,,"
        Option "XkbOptions" "grp:alt_shift_toggle,ctrl:nocaps"
EndSection

Press Alt_L and Shift_L to change from keyboard layout us (QWERTY) to de (QWERTZ).

before, group = 1
Intercepted key event 2, key code 47
after, group = 1
;before, group = 1 // pressing semicolon key
Intercepted key event 3, key code 47
after, group = 1
before, group = 1
Intercepted key event 2, key code 64 // alt key
after, group = 1
before, group = 2 // group already changed to 2 (0x4000, de QWERTZ)
Intercepted key event 2, key code 50 // shift key
after, group = 2
before, group = 2
Intercepted key event 3, key code 50
after, group = 2
before, group = 2
Intercepted key event 3, key code 64
after, group = 2
before, group = 2
Intercepted key event 2, key code 47
after, group = 2
öbefore, group = 2 // press semicolon key, now an "ö" (de QWERTZ)
Intercepted key event 3, key code 47
after, group = 2

Solution

In the first case the group change appears at the end of intercept(), whereas in the second case the group change occurs before intercept(). Saving the group from the previous key event therefore allows to separate the two cases and handle them correctly. If user_data->previous_group matches current_group at the beginning of intercept(), the group will be set to user_data->intended_group. If user_data->previous_group does not match current_group, both user_data->intended_group and user_data->previous_group will be set to current_group to confirm the layout change.

Copy link
Owner

@alols alols left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@@ -413,6 +424,18 @@ void intercept (XPointer user_data, XRecordInterceptData *data)
}
}

if (self->previous_group != current_group)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like previous_group is read before written to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous_group is now initialized to -1. The first time intercept() is called the condition in this "if" will definitely be true. Since the bug did occur only after intercept() was already called for the key press event, this case will work exactly as before. If switching to another keyboard change was set to a single key (for example grp:lalt_toggle), the initialization of previous_group to intended_group might have caused problems since the keyboard group is changed before intercept() is called.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While writing the comment my first commit started to make more and more sense. It will also handle both cases correctly and it seems to be a more reasonable solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants