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

lowlevel_keyboard_event_data.h doesn't seem to swallow keyboard events #1944

Closed
JordanAnthonyKing opened this issue Apr 5, 2020 · 5 comments
Labels
Product-FancyZones Refers to the FancyZones PowerToy

Comments

@JordanAnthonyKing
Copy link
Contributor

JordanAnthonyKing commented Apr 5, 2020

Environment

Windows build number: 19041.172
PowerToys version: Master/v0.16.0
PowerToy module for which you are reporting the bug: Interface

Steps to reproduce

I've been playing around with the source code for FancyZones a lot recently. When adding keybindings the header file interface/lowlevel_keyboard_event_data.h is used to prevent keyboard events being sent to the OS. I don't believe this functionality actually works. An example given to block Win+L in the file is:

virtual intptr_t signal_event(const wchar_t* name, intptr_t data) override {
    if (wcscmp(name, ll_keyboard) == 0) {
      auto& event = *(reinterpret_cast<LowlevelKeyboardEvent*>(data));
      // The L key has vkCode of 0x4C
      if (event.wParam ==  WM_KEYDOWN && event.lParam->vkCode == 0x4C) {
        return 1;
      } else {
        return 0;
      }
    } else {
      return 0;
    }
  }

Testing this out by altering the code in FancyZones.cpp like thus:

IFACEMETHODIMP_(bool)
FancyZones::OnKeyDown(PKBDLLHOOKSTRUCT info) noexcept
{
    // Return true to swallow the keyboard event
    bool const shift = GetAsyncKeyState(VK_SHIFT) & 0x8000;
    bool const win = GetAsyncKeyState(VK_LWIN) & 0x8000;
    if (win && !shift)
    {
        if ((info->vkCode == VkKeyScan('l')) || (info->vkCode == VkKeyScan('h'))) // <-- h & l hotkeys like vim
        {
            if (m_settings->GetSettings()->overrideSnapHotkeys)
            {
                Trace::FancyZones::OnKeyDown(info->vkCode, win, ctrl, false /*inMoveSize*/);
                return OnSnapHotkey(info->vkCode);
            }
        }
    }
    if (m_dragEnabled && shift)
    {
        return true;
    }
    return false;
}

OnSnapHotkey has been updated accordingly.

Pressing Win+L locks the computer. The window does move accordingly and placing a breakpoint in the if statement shows that it does get hit. The keyboard event just isn't being swallowed as it should.

Win+L is notoriously difficult to block, though I have found this affects other keys too, like Win+plus which is also not blocked and opens the magnifier.

Expected behavior

The defined key presses should not be passed to the OS.

Actual behavior

Key presses are being passed to the OS preventing them from being effectively rebound.

@JordanAnthonyKing JordanAnthonyKing changed the title lowlevel_keyboard_event_data.h doesn't seem to swallow keyboard events. lowlevel_keyboard_event_data.h doesn't seem to swallow keyboard events Apr 5, 2020
@arjunbalgovind
Copy link
Contributor

@JordanAnthonyKing Win+L cannot be suppressed by a low level hook, but Win+Plus and Win+Minus should get suppressed by the hook. Can you post the code snippet of how you handled Win+Plus?

@JordanAnthonyKing
Copy link
Contributor Author

JordanAnthonyKing commented Apr 6, 2020

@arjunbalgovind You're right, Win+Plus is blocked properly, I likely made a mistake when I first gave it a go.

Thanks for letting me know that Win+L cannot be unbound. Perhaps the comment in lowlevel_keyboard_event_data.h should be changed, though I guess it isn't talking about Win+L specifically. Is there a resource for which key combinations cannot be blocked?

Shall I close the issue?

@arjunbalgovind
Copy link
Contributor

I don't have a proper resource about which key combinations can't be blocked, apart from stack overflow answers and just trial and error. While working on the Keyboard Manager(#6) we have found that Win+L and Ctrl+Alt+Del can't be suppressed. These are two that we are aware of right now. We will be looking into other shortcuts later, but most of the Win shortcuts that we have been testing with seem to get suppressed with low level hooks.

Shall I close the issue?

@crutkas I'll defer that to you.

@crutkas
Copy link
Member

crutkas commented Apr 6, 2020

I defer to @enricogior and his team here.

@crutkas crutkas added the Product-FancyZones Refers to the FancyZones PowerToy label Apr 6, 2020
@enricogior
Copy link
Contributor

That code has been working consistently since version 0.11, for existing module it hasn't caused any problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-FancyZones Refers to the FancyZones PowerToy
Projects
None yet
Development

No branches or pull requests

4 participants