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

add xdotool type --aftermodifiers option #406

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

Conversation

scgtrp
Copy link

@scgtrp scgtrp commented Oct 15, 2022

This waits for the user to release any modifier keys that are currently held before starting to type. This allows avoiding race conditions when xdotool is called from a keyboard shortcut, and starts to type characters with the modifier still held. Currently the usual way to do so is with --clearmodifiers, but this introduces its own race conditions (see e.g. issue #43) if the physical key is released while the typing is in progress.

This waits for the user to release any modifier keys that are currently held before starting to type. This allows avoiding race conditions when xdotool is called from a keyboard shortcut, and starts to type characters with the modifier still held. Currently the usual way to do so is with --clearmodifiers, but this introduces its own race conditions (see e.g. issue jordansissel#43) if the physical key is released while the typing is in progress.
@scgtrp
Copy link
Author

scgtrp commented Oct 15, 2022

Hmm, the same issue probably also applies to xdotool key et al. If this approach is okay with you, I'll go back and add those too (and also update the manpage).

@jordansissel
Copy link
Owner

jordansissel commented Oct 15, 2022 via email

@phil294
Copy link
Contributor

phil294 commented Oct 15, 2022

I coincidentally stumbled upon this problem too today, however one thing I don't understand @scgtrp:

avoiding race conditions when xdotool is called from a keyboard shortcut, and starts to type characters with the modifier still held

this is true, but I have noted this has nothing to do with modifier keys. It seems that xdotool cannot send any key, while the issuing shortcut key is still held. In particular, this means e.g. when you configure a desktop environment shortcut on key press a with its action being xdotool type b, this won't work! The shortcut is grabbed, but nothing is typed. I wonder why this PR solves the issue for you. Perhaps it's specific to XFCE though?

In other words, what kind of desktop environment are you using?

@scgtrp
Copy link
Author

scgtrp commented Oct 15, 2022

My DE is xfce + i3wm.

The specific issue that led me here was fdw/rofi-rbw#56 - as far as I know, that's not a grabbed key binding, but just a normal key sequence going into that application's window that happens to close the window and execute xdotool in quick succession before I can release the alt key.

I did just try binding a key to this in the XFCE application shortcuts thing, though, and it seems to work. Specifically, I bound alt-shift-H to /usr/local/bin/xdotool type --aftermodifiers hello, then did that a few times into this text box, releasing the modifier keys slowly in both orders. Both times, it typed "hello" in lowercase after releasing both keys. Do you have an example of a binding that doesn't work?

@phil294
Copy link
Contributor

phil294 commented Oct 16, 2022

ah, I see now. In your example, the grabbing problem I described doesn't apply because by use of --aftermodifiers, it waits for ctrl and shift, after the releasing of which the H key itself has also already been lifted. No key is pressed anymore and thus any typing succeeds.

Do you have an example of a binding that doesn't work?

  • a -> xdotool type b: Pressing a types nothing
  • h -> xdotool type hello: Pressing h types ello
  • h -> xdotool type ello: Pressing h types nothing

However, I should probably open up a new issue for this. In my project which does also some grabbing, I solved this now by going the route of a -> xdotool keyup a; xdotool type b.

Regarding your PR, I think it's definitely useful and should be merged. As I understand it, you're waiting and polling for the pressed modifiers 33 times a second. This seems like a reasonable approach, unless X11 offers another api that doesn't involve polling.

@scgtrp
Copy link
Author

scgtrp commented Oct 16, 2022

Ah, yes, I can almost reproduce that: for me, any xdotool type command bound to a letter key without modifiers fails. That's annoying. I think that may still be solveable with this approach, though: just have it wait for all keys to be released, regardless of whether they're modifiers. That also solves the similar problem that happens in (press modifier -> press letter -> release modifier -> exec xdotool -> release letter).*

I actually settled on this after getting halfway through a much more overengineered solution involving grabbing all the modifier keys and waiting for KeyRelease events on them. I'm not actually sure it would have worked, and there's precedent for the polling-every-30ms approach in the other xdo_wait_for_... functions.

(* which I discovered last night by idly wondering if I could bind a command to shift-letter, trying it, forgetting I'd done so, and then later trying to start an email with "Hello" and getting the much less formal "ELLOello" instead)

@jordansissel
Copy link
Owner

the grabbing problem I described

@phil294 I have a hypothesis that this was usually due to the way bindings work with XGrabKeyboard and XUngravKeyboard — but I don’t remember if I’ve tested that idea.

That said, I’d welcome an issue about the problem you’re having — maybe there’s a better way to solve this than what I’ve recommended historically which is to add a short sleep before sending keys/typing?

like maybe xdotool could detect if the x11 keyboard is grabbed and wait until it’s ungrabbed? I haven’t looked at this in a while, which is a good reason as any to revisit! ;)

@scgtrp
Copy link
Author

scgtrp commented Oct 16, 2022

So I just hacked together a --afterkeys that does the same thing as --aftermodifiers but for all keys. Seems to work, with two half-caveats: my X server was convinced I was holding down the Hiragana_Katakana key (which I don't have) and I had to go xdotool keyup Hiragana_Katakana to get --afterkeys to work. Unsure if that's a common thing that nobody ever notices because nobody has a Hiragana_Katakana key unless they actually type hiragana and katakana, or just a horrible misconfiguration of my keymap somewhere.

The other half-caveat is that I managed to forkbomb myself by binding h to xdotool type --afterkeys hello. Turns out if the xdotool-generated h happens after the user-entered h is released, that counts as a new keypress and can trigger the binding again. So, uh, don't do that, I guess. :)

I've pushed that change here so people can play with it.

@jordansissel
Copy link
Owner

It’s still on my todo list to play with this more. I haven’t used my Linux workstation in a few weeks so it may be some time before I get to test. I’d welcome any others to test and provide feedback about the behavior improved by this patch!

@fdw
Copy link

fdw commented Nov 27, 2022

This sounds like a really good way to solve that problem, so I'm waiting for the merge 😉

The other half-caveat is that I managed to forkbomb myself by binding h to xdotool type --afterkeys hello. Turns out if the xdotool-generated h happens after the user-entered h is released, that counts as a new keypress and can trigger the binding again. So, uh, don't do that, I guess. :)

That, however, doesn't sound so good. For example, in the mentioned rofi-rbw, I know neither the shortcut nor the typed letters, so it's impossible to make sure that this doesn't happen. Wouldn't it suffice to check that all keys are release once, and then not check again? It seems that the current implementation is less of a --afterkeys and more of --no-other-keys.

@scgtrp
Copy link
Author

scgtrp commented Nov 27, 2022

Wouldn't it suffice to check that all keys are release once, and then not check again? It seems that the current implementation is less of a --afterkeys and more of --no-other-keys.

It does wait only once, unless I'm misunderstanding what you're saying here.

I suspect (but haven't checked) that this can happen today, if you have a binding on a bare character or shift+character and that character is in the password you selected. I'm not really sure what to do about that, because there's no mechanism to make an xdotool type --bypassing-any-grabbed-keys that I'm aware of.


Unrelated to that, it may be cleaner to add an xdotool waitforkeyrelease instead of adding a flag to every command that might need it.

@fdw
Copy link

fdw commented Nov 28, 2022

Wouldn't it suffice to check that all keys are release once, and then not check again? It seems that the current implementation is less of a --afterkeys and more of --no-other-keys.

It does wait only once, unless I'm misunderstanding what you're saying here.

What I meant was that once all keys have been released, it should not lock/wait again. For example, if the keybinding is h, after h has been released, it would send all characters, including h, without checking for a needed release.
Does that make more sense?

@scgtrp
Copy link
Author

scgtrp commented Nov 28, 2022

It does that (with all keys, not just the keybinding, because it doesn't know what the keybinding was):

    if (after_keys) {
      xdo_wait_for_key_release(context->xdo);
    }

    // ...

    for (i = 0; i < data_count; i++) {
      // type some text
    }

phil294 added a commit to phil294/AHK_X11 that referenced this pull request Jul 20, 2023
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.

4 participants