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

Don't send repeat key presses to clients #94

Closed
wants to merge 10 commits into from

Conversation

iacore
Copy link

@iacore iacore commented Feb 24, 2022

Attempt to fix QubesOS/qubes-issues#7231 partially.

Not tested due to GLIBC version mismatch.

@iacore
Copy link
Author

iacore commented Feb 25, 2022

New problem: fedora32 templates don't generate key repeat events, while archlinux do.

@marmarek
Copy link
Member

New problem: fedora32 templates don't generate key repeat events, while archlinux do.

Check if it isn't disabled via xset.
Anyway, that's worrying, because even if we enable it with gui-agent-linux update, old versions will basically have broken key repeat functionality. Plus, non-Linux systems (in HVM) may misbehave too.

I think a cleaner solution is the initial idea: new message with flags included, instead of changing behavior of existing message.

@iacore
Copy link
Author

iacore commented Feb 25, 2022

Check if it isn't disabled via xset.

I enabled it via xset and it now works.

I've installed the last commit "dedd3f6" in dom0 and it works fine.

I think just dropping repeated key events is better. The client OS should generate its own key repeat events. At least for keyboard I don't need other flags than repeat. This way, I don't need to change the linux gui agent for it to work. Just enabling key repeat with xset is good enough.

I think archlinux templates will generate its own set of repeat events (so double the repeat speed) even when fed with repeat events.

@iacore
Copy link
Author

iacore commented Feb 25, 2022

Also, building this in fedora 32 now requires package libXi-devel (for CI to pass). How do I add that?

@marmarek
Copy link
Member

I mean, send events including the (repeat) flag and let the gui agent decide. It can either:

  1. ignore repeat events and let local X server generate them
  2. disable repeat in local X server and use those sent by gui-daemon

Both have some pros and cons, with 2nd option respecting your globally configured repeat speed - that's why I think it is a better solution. But if going with the former, it should be at least opt-in (can be via config option), or combined with some mechanism ensuring VM actually have its own key repeat enabled (probably using qvm-feature mechanism). Otherwise it will break some existing configurations.

Also, building this in fedora 32 now requires package libXi-devel (for CI to pass). How do I add that?

Add to rpm_spec/gui-daemon.spec.in, and also to debian/control.

@iacore
Copy link
Author

iacore commented Mar 3, 2022

New problem: when switching windows from window A to window B using "Super + Tab", the tab key press was sent to window A.

This is not a logic bug, but it's annoying. For example, when switching away from Firefox, "Tab" scroll the page down.

Here's what captured by xinput test-xi2 when using "Super + Tab" to switch to another window, from the perspective of the previous window:

EVENT type 13 (RawKeyPress)
    device: 3 (20)
    detail: 133
    valuators:

EVENT type 2 (KeyPress)
    device: 3 (20)
    detail: 133
    flags: 
    root: 1290.00/111.00
    event: 430.00/-325.00
    buttons:
    modifiers: locked 0x10 latched 0 base 0 effective: 0x10
    group: locked 0 latched 0 base 0 effective: 0
    valuators:
    windows: root 0x7ca event 0x4800001 child 0x0
EVENT type 13 (RawKeyPress)
    device: 3 (20)
    detail: 23
    valuators:

EVENT type 10 (FocusOut)
    device: 3 (3)
    windows: root 0x7ca event 0x4800001 child 0x0
    mode: NotifyGrab (detail NotifyAncestor)
    flags:  [same screen]
    buttons:
    modifiers: locked 0x10 latched 0 base 0x40 effective: 0x50
    group: locked 0 latched 0 base 0 effective: 0
    root x/y:  1290.00 / 111.00
    event x/y: 430.00 / -325.00
EVENT type 14 (RawKeyRelease)
    device: 3 (20)
    detail: 133
    valuators:

EVENT type 9 (FocusIn)
    device: 3 (3)
    windows: root 0x7ca event 0x4800001 child 0x0
    mode: NotifyUngrab (detail NotifyAncestor)
    flags:  [same screen]
    buttons:
    modifiers: locked 0x10 latched 0 base 0 effective: 0x10
    group: locked 0 latched 0 base 0 effective: 0
    root x/y:  1290.00 / 111.00
    event x/y: 430.00 / -325.00
EVENT type 10 (FocusOut)
    device: 3 (3)
    windows: root 0x7ca event 0x4800001 child 0x0
    mode: NotifyNormal (detail NotifyNonlinear)
    flags:  [same screen]
    buttons:
    modifiers: locked 0x10 latched 0 base 0 effective: 0x10
    group: locked 0 latched 0 base 0 effective: 0
    root x/y:  1290.00 / 111.00
    event x/y: 430.00 / -325.00
EVENT type 14 (RawKeyRelease)
    device: 3 (20)
    detail: 23
    valuators:

detail: 133 = Super
detail: 23 = Tab

Only client VM windows behave weirdly.

This is probably XInput's input focus being different from the legacy X basic window focus events.
https://www.x.org/releases/current/doc/libXi/inputlib.html#Events

@iacore
Copy link
Author

iacore commented Mar 4, 2022

This solution doesn't work well. I'll try using XInput for all input events instead.

@iacore iacore closed this Mar 4, 2022
@iacore iacore mentioned this pull request Mar 4, 2022
3 tasks
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.

gui rpc drop keystroke flags like repeat
2 participants