mumble-voip / mumble

Mumble is an open-source, low-latency, high quality voice chat software.
https://www.mumble.info
Other
6.26k stars 1.11k forks source link

a11y: Mumble keyboard hook never passes keys to earlier keyboard hooks #2217

Open jcsteh opened 8 years ago

jcsteh commented 8 years ago

Screen readers use low level keyboard hooks to respond to keys pressed in applications (e.g. interrupting speech when a key is pressed) and to implement their own global keyboard commands. In the former case, the keys are noted but allowed through the hook. In the latter case, the hook traps the keys. It seems that when Mumble starts, a screen reader's hook no longer gets notified about pressed keys within Mumble, even if Mumble doesn't specifically need to trap a key. This means a screen reader user cannot use screen reader commands within Mumble.

If you restart the screen reader after starting Mumble, everything works again, as the screen reader's hook now takes precedence. Despite this workaround, it'd be good if this could be fixed.

STR:

  1. In Windows 8.1 or later, start Narrator by pressing Windows key+enter.
  2. Start Mumble.
  3. Press caps lock+f1; i.e. hold down caps lock, press f1 and release caps lock. This is a Narrator command.
    • Expected: Narrator should display a window called "Search all commands".
    • Actual: Nothing happens.
  4. Exit Narrator by pressing Windows+enter.
  5. Start Narrator again by pressing Windows+enter.
  6. Ensure Mumble is focused.
  7. Press caps lock+f1.
    • Result: The "Search all commands" window appears.

Suggested fix: Ensure the keyboard hook returns CallNextHookEx if it doesn't explicitly need to trap the key.

Tested version: 1.3.0~982~gef72e3e~snapshot

mkrautz commented 8 years ago

If the key isn't suppressed, that should be exactly what happens already: https://github.com/mumble-voip/mumble/blob/master/src/mumble/GlobalShortcut_win.cpp#L286

Am I missing something?

One thing that might confuse existing accessibility tools is that Mumble has uiAccess=true in its manifest. So to Windows, Mumble is itself treated as an accessibility tool. Maybe that causes problems?

The reason we do it, is to improve the user experience by allow Mumble to receive keypresses from elevated programs (such as some MMO games, but also just elevated command prompts, and what have you...).

jcsteh commented 8 years ago

Hmm. It certainly seems that should be happening already, yes. I assume gsw->handleButton should only return true if it was a defined shortcut? I don't quite follow when g.ocIntercept will evaluate to true. The comment says "In full-GUI-overlay always suppress". I assume that won't apply by default, though?

Having uiAccess true shouldn't cause this. If I start Narrator while NVDA is running (both of which have uiAccess), Narrator eats its own keystrokes, but doesn't eat NVDA's, and NVDA can still see keystrokes destined for foreground apps (which neither NVDA nor Narrator eat).

mkrautz commented 8 years ago

Yes, g.ocIntercept isn't used in practice anymore, so we can disregard that.

gsw->handleButton should only return true if the user has chosen to suppress the shortcut. (There's a suppress checkbox next to shortcuts in Mumble's shortcut settings -- users use it to ensure that when they press their shortcut key, it doesn't propagate to any applications or games.)

So, in most cases, it shouldn't return true. Unless a user has explicitly chosen to suppress the given shortcut.

jcsteh commented 8 years ago

I haven't chosen to suppress any shortcuts. I guess I'll need to build and debug at some point. Something weird is definitely going on here; as noted above, this doesn't happen with other uiAccess apps with keyboard hooks.

mkrautz commented 8 years ago

BTW, it might be interesting to run Mumble without uiAccess.

The easiest way I know to accomplish that without too much trouble is to start mumble.exe from a command prompt:

set __COMPAT_LAYER=RunAsInvoker mumble.exe

With __COMPAT_LAYER=RunAsInvoker set, Mumble will run w/o uiAccess.

It's also possible to disable the mouse and keyboard hooks via the registry: http://mumble.info/reg/disable-winhooks.reg

As a workaround, and I would imagine, a great QoL improvement for people using screen readers, I suppose Mumble could disable its mouse/keyboard hooks (if that works) if it detects a screen reader running. I don't know if there's a programmatic way to determine that?

jcsteh commented 8 years ago

Thanks for the "set __COMPAT_LAYER=RunAsInvoker" tip. I never knew about that despite being one of the lead developers of the NVDA screen reader! :)

Running without uiAccess doesn't fix this. (I know it's definitely not running with uiAccess when I try this because a copy of NVDA without uiAccess can't read it at all.)

If I disable the keyboard hook, I imagine my push to talk, etc. shortcuts won't work? I (and I imagine other screen reader users) depend on those, so if they do stop working, that isn't a feasible workaround.

Btw, thanks so much for the quick responses... and the great software!

mkrautz commented 8 years ago

Mumble will fall back to DirectInput for global shortcuts when hooks are disabled. The downside that one can't suppress shortcuts in that mode. (And shortcuts may not work when in elevated programs. I don't actually know!)

So, a bit of a downgrade. But not everyone uses shortcut suppression.

jcsteh commented 8 years ago

I shall investigate. I'm still pretty sure something is not doing what it should here, but I'll have to wait until I have time to build and debug to figure it out.

wizpig64 commented 8 years ago

I have a related issue that seems related but works a little differently. The workaround above fixed it for me, and setting it as an environment variable made the change permanent so I don't have to run mumble out of a batch script thankfully. Here's the ticket I was going to submit before finding this one:

The shortcuts menu has a checkbox for each shortcut that prevents other applications from seeing that keypress. When checked, it works no matter what window you have selected. When unchecked, it still suppresses that keypress for other windows if mumble is the active window. I don't know if this is intended behavior or not.

Here's the problem with this: If I'm muted in mumble and am having a push-to-talk conversation with someone in another voip program, my voice is interrupted or stuck in the on position when I click into mumble. I would like an option to disable this functionality entirely when suppression is unchecked, assuming that the above behavior is intended.

I'm using 1.3.0 snapshot build g8bdfd7d on Windows 10 x64 v1607, the latest for each at time of writing. I tested this with 1.2.16 as well and it happens there too.

Thanks.

Krzmbrzl commented 4 years ago

Does this still happen in Mumble 1.3 (the released version)?