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

Crash in KeyboardEventHandler when no corewindow #8063

Closed
Tracked by #9393
asklar opened this issue Jun 18, 2021 · 3 comments · Fixed by #10859
Closed
Tracked by #9393

Crash in KeyboardEventHandler when no corewindow #8063

asklar opened this issue Jun 18, 2021 · 3 comments · Fixed by #10859

Comments

@asklar
Copy link
Member

asklar commented Jun 18, 2021

  • run template app against winui 3
  • hover over something, press the alt key
  • we crash when dereferencing a null corewindow:

note that there are more usages of corewindow in this file that we need to provide alternatives when no corewindow is available

void UpdateModifiedKeyStatusTo(T &event) {
auto const &coreWindow = winrt::CoreWindow::GetForCurrentThread();
event.altKey = KeyboardHelper::IsModifiedKeyPressed(coreWindow, winrt::Windows::System::VirtualKey::Menu);
event.shiftKey = KeyboardHelper::IsModifiedKeyPressed(coreWindow, winrt::Windows::System::VirtualKey::Shift);
event.metaKey = KeyboardHelper::IsModifiedKeyPressed(coreWindow, winrt::Windows::System::VirtualKey::LeftWindows) ||
KeyboardHelper::IsModifiedKeyPressed(coreWindow, winrt::Windows::System::VirtualKey::RightWindows);
event.ctrlKey = KeyboardHelper::IsModifiedKeyPressed(coreWindow, winrt::Windows::System::VirtualKey::Control);
event.capLocked = KeyboardHelper::IsModifiedKeyLocked(coreWindow, winrt::Windows::System::VirtualKey::CapitalLock);

@asklar asklar added the bug label Jun 18, 2021
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Jun 18, 2021
@asklar asklar added Area: WinUI and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Jun 24, 2021
@asklar asklar added this to the 0.66 milestone Jun 24, 2021
@asklar asklar self-assigned this Jun 24, 2021
@chrisglein chrisglein modified the milestones: 0.66, 0.67 Aug 31, 2021
@asklar asklar modified the milestones: 0.67, 0.68 Oct 22, 2021
@asklar
Copy link
Member Author

asklar commented Oct 22, 2021

will need to try this out in winappsdk 1.0 once we update
@AgneLukoseviciute fyi

@jonthysell
Copy link
Contributor

@TatianaKapos Do you think this will make 0.69 or should it be moved to 0.70?

@TatianaKapos TatianaKapos modified the milestones: 0.69, 0.70 May 9, 2022
@TatianaKapos TatianaKapos modified the milestones: 0.70, Backlog Aug 3, 2022
@asklar asklar modified the milestones: Backlog, 0.71 Aug 11, 2022
@lyahdav
Copy link
Collaborator

lyahdav commented Sep 16, 2022

Heads up: I have a fix for this. Will put up PR shortly.

jonthysell pushed a commit that referenced this issue Nov 15, 2022
…ng unsupported class (#10859)

[Per WinAppSDK the CoreWindow class is unsupported](https://docs.microsoft.com/en-us/windows/apps/desktop/modernize/desktop-to-uwp-supported-api#core-unsupported-classes). It's used in KeyboardEventHandler for calling GetKeyState, which the docs provide an alternative:
> Instead of the [GetKeyState](https://docs.microsoft.com/en-us/uwp/api/windows.ui.core.corewindow.getkeystate) method, use the [InputKeyboardSource.GetKeyStateForCurrentThread](https://docs.microsoft.com/en-us/windows/windows-app-sdk/api/winrt/microsoft.ui.input.inputkeyboardsource.getkeystateforcurrentthread) method provided by WinUI 3 instead.

This PR makes that change.

Resolves #8063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment