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 UnhandledKeyPressed.md #3859

Open
wants to merge 12 commits into
base: api-UnhandledKeyPressed
Choose a base branch
from
234 changes: 234 additions & 0 deletions specs/UnhandledKeyPressed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@

# Background
Consumers of the old [WebBrowser](https://learn.microsoft.com/en-us/dotnet/api/system.windows.controls.webbrowser?view=windowsdesktop-7.0) control that relied on the [OnKeyDown](https://learn.microsoft.com/previous-versions/aa752133(v=vs.85)) API that allowed them to receive and handle key events not handled by the browser, [requested](https://github.com/MicrosoftEdge/WebViewFeedback/issues/468) same ability in WebView2.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please limit line lengths to 80 or 100 characters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OnKeyDown links to IWebBrowser2::Navigate. Is it supposed to link to an OnKeyDown page?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. Fixed it.


# Description
The `UnhandledKeyPressed` event allows developers to subscribe event handlers to be run when a key event is not handled by the browser (including DOM and browser accelerators).

`UnhandledKeyPressed` event can be triggered by all keys, which is different from [AcceleratorKeyPressed](https://learn.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2acceleratorkeypressedeventargs?view=webview2-dotnet-1.0.705.50) event.

`UnhandledKeyPressed` event is async, which means [GetKeyState](https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getkeystate) does not return the exact key state when the key event is fired. Use `UnhandledKeyPressedEventArgs.Modifiers` instead to verify whether Ctrl or Alt is down in this situation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please limit line lengths to no more than 80 or 100 characters.


# Examples
## C++

```cpp
auto controller5 = m_controller.try_query<ICoreWebView2Controller5>();
if (controller5)
{
CHECK_FAILURE(controller5->add_UnhandledKeyPressed(
Callback<ICoreWebView2UnhandledKeyPressedEventHandler>(
[this](
ICoreWebView2Controller* sender,
ICoreWebView2UnhandledKeyPressedEventArgs* args) -> HRESULT
{
COREWEBVIEW2_KEY_EVENT_KIND kind;
CHECK_FAILURE(args->get_KeyEventKind(&kind));

// We only care about key down events.
if (kind == COREWEBVIEW2_KEY_EVENT_KIND_KEY_DOWN ||
kind == COREWEBVIEW2_KEY_EVENT_KIND_SYSTEM_KEY_DOWN)
{
COREWEBVIEW2_KEY_PRESSED_FLAG_KIND flag;
CHECK_FAILURE(args->get_Modifiers(&flag));
if (flag & COREWEBVIEW2_KEY_EVENT_FLAG_CONTROL_DOWN)
{
UINT key;
CHECK_FAILURE(args->get_VirtualKey(&key));
// Check if the key is one we want to handle.
if (key == 'Z')
{
MessageBox(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure the C# and C++ code samples do the same thing. Here the C++ does more checking for other Ctrl+A-Z that C# doesn't and C++ is showing a message box while C# is writing to debug out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Updated the C# code to sync with C++ code.

nullptr,
L"Key combination Ctrl + Z unhandled by browser is "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think that normally Ctrl+Z is an accelerator that the browser would handle by undoing text typed into a textbox. Will this only be raised if there is nothing to undo? Or does it get raised regardless? It would be good to explain in a comment in the code samples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be raised only when there is nothing to undo. Added a comment here.

L"triggered.",
L"", MB_OK);
}
else if (key >= 'A' && key <= 'Z')
{
OutputDebugString((std::wstring(L"Ctrl +") + (wchar_t)key +
L" not handled by browser is triggered.")
.c_str());
}
}
}
return S_OK;
})
.Get(),
&m_unhandledKeyPressedToken));
}
```

## C#

```csharp
private CoreWebView2Controller controller;
void RegisterKeyEventHandlers()
{
if (controller != null)
{
controller.UnhandledKeyPressed += CoreWebView2Controller_UnhandledKeyPressed;
}
}
void CoreWebView2Controller_UnhandledKeyPressed(object sender, CoreWebView2UnhandledKeyPressedEventArgs e)
{
if ((e.KeyEventKind == CoreWebView2KeyEventKind.KeyDown) && (e.Modifiers & CoreWebView2KeyPressedFlagKind.CoreWebView2KeyEventFlagControlDown) != 0)
{
if (e.VirtualKey == 'z')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says this is an upper case letter not lower case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. Fixed it.

{
Debug.WriteLine($"Key combination Ctrl + Z unhandled by browser is triggered.");
}
}
}
```

# API Details

## Win32 C++
```idl
[uuid(053b9a5d-7033-4515-9898-912977d2fde8), object, pointer_default(unique)]
interface ICoreWebView2Controller : IUnknown {
/// Adds an event handler for the `UnhandledKeyPressed` event.
/// `UnhandledKeyPressed` runs when an key is not handled in
/// the DOM.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to explicitly say that this event will be raised after WebView2 has a chance to handle the key combination including the DOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified the comment here to make it more clear here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this would also not be raised if the Accelerator event marks this handled? Or if its an accelerator that WebView2 handles like Ctrl+P or Ctrl+X? And how does this relate to the DOM key event handler in script? If the DOM event marks the key press handled or not will result in the key press being 'unhandled' and raising this UnhandledKeyPressed event? It would be good to give examples.


HRESULT add_UnhandledKeyPressed(
[in] ICoreWebView2UnhandledKeyPressedEventHandler* eventHandler,
[out] EventRegistrationToken* token);

/// Removes an event handler previously added with
/// `add_UnhandledKeyPressed`.

HRESULT remove_UnhandledKeyPressed(
[in] EventRegistrationToken token);
}

[uuid(dc83113b-ce7a-47bb-9661-16b03ff8aac1), object, pointer_default(unique)]
interface ICoreWebView2UnhandledKeyPressedEventHandler : IUnknown {

/// Provides the event args for the corresponding event.

HRESULT Invoke(
[in] ICoreWebView2Controller* sender,
[in] ICoreWebView2UnhandledKeyPressedEventArgs* args);
}

[uuid(dd30e20c-d1b3-4e51-9bb6-1be7aaa3ce90), object, pointer_default(unique)]
interface ICoreWebView2UnhandledKeyPressedEventArgs : IUnknown {

/// The key event type that caused the event to run.

[propget] HRESULT KeyEventKind([out, retval] COREWEBVIEW2_KEY_EVENT_KIND* keyEventKind);

/// The Win32 virtual key code of the key that was pressed or released. It
/// is one of the Win32 virtual key constants such as `VK_RETURN` or an
/// (uppercase) ASCII value such as `A`.
/// use ICoreWebView2UnhandledKeyPressedEventArgs::get_Modifiers()
/// instead of Win32 API ::GetKeyState() to verify whether Ctrl or Alt
/// is pressed.

[propget] HRESULT VirtualKey([out, retval] UINT* virtualKey);

/// The `LPARAM` value that accompanied the window message. For more
/// information, navigate to [WM_KEYDOWN](/windows/win32/inputdev/wm-keydown)
/// and [WM_KEYUP](/windows/win32/inputdev/wm-keyup).
/// For visual hosting, only scan code and extended key (16-24bit) are provided.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this the case? AcceleratorKeyPressed does not have this restriction for visual hosting.

Copy link
Contributor Author

@zhuhaichao518 zhuhaichao518 Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For visual hosting, the information of the native part (wparam, lparam, including these bits) was dropped when sent to WebView. This is not an issue for visual hosting because browser does not care about these bits (other bits were converted to a cross-platform part). If we need to get the exact lparam at UnhandledKeyPressed, we need to make some changes on how key events are sent to browser for visual hosting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, is this because keyboard is not using the same mojo path as mouse and touch? Mouse and touch do send across the w_param and l_param. We should fix this so this API can return the correct l_param.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another case for making this change, is if the LParam is missing values, then the COREWEBVIEW2_PHYSICAL_KEY_STATUS should also be missing those values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because we are reusing the mojom type which does not include native part of the input event. It was originally used by browser to send input events to render process, so it drops the native part (I think it is by design and we should not send the native info to renderer). We create the event on the host process so the native info was dropped when sent to browser process. Maybe we can send a copy of the native part to browser process to resolve this.

/// For other bits, check out the origin message from ::GetMessage().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to imply that the app can call GetMessage to retrieve the information. Even if the app saved the MSG from the call to GetMessage, how would they match that to an UnhandledKeyPressed event since the event is async from the MSG being received?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devs should expect which keys should fire UnhandledKeyPressed by themselves to know which message the UnhandledKeyPressed is related to. It is not very healthy. I did not expect it to be used often, the cases they need to do it is when they need the repeat count and previous key state according to the doc. Maybe we can make some changes to on how key events are sent to browser for visual hosting to avoid this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sample code uses Ctrl+Z as an example of a key combination that sometimes is handled and sometimes unhandled by the browser. If the user presses Ctrl+Z twice, the app might only get 1 unhandled Ctrl+Z or it might get 2. It would depend on the content of the text box. When the first unhandled Ctrl+Z comes in, the app wouldn't know which GetMessage to use. As I commented above, we should fix keyboard to use the same path as mouse and touch which does include the w_param and l_param.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, what I mean was the dev should keep everything including how many characters users are putting in a textbox -- that includes lot of work on the js side and it is not healthy.
After weighing the use case and the cost of modification, I decided not to change the event and added a comment. Fixing this by making some modification to the event is fine to me.


[propget] HRESULT KeyEventLParam([out, retval] INT* lParam);

/// A structure representing the information passed in the `LPARAM` of the
/// window message.

[propget] HRESULT PhysicalKeyStatus(
[out, retval] COREWEBVIEW2_PHYSICAL_KEY_STATUS* physicalKeyStatus);

/// The `Handled` property will not influence the future WebView
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this event need a Handled property? The sample code doesn't show using Handled. At this point, WebView2 has already decided not to do anything with the key. How does setting Handled in this event impact WebView2 behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it can be used when devs register more than one UnhandledKeyPressed handler and they do not want to handle they event on the second handler if the previous handler has marked the event as handled.

/// action. It can be used with other UnhandledKeyPressed
/// Event handlers.

[propget] HRESULT Handled([out, retval] BOOL* handled);

/// Sets the `Handled` property.

[propput] HRESULT Handled([in] BOOL handled);

/// Retrieves the status of the keyboard when the key event is triggered.
/// Use this instead of ::GetKeyState().
/// See COREWEBVIEW2_KEY_PRESSED_FLAG_KIND for details.
[propget] HRESULT Modifiers([out, retval] COREWEBVIEW2_KEY_PRESSED_FLAG_KIND* modifiers);
}

/// Flag bits representing the state of keyboard when a "UnhandledKeyPressed" Event happens.
[v1_enum]
typedef enum COREWEBVIEW2_KEY_PRESSED_FLAG_KIND {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the values of these match internal Chromium values. Other enum values around keyboard/mouse are based on publicly documented Windows values which are guaranteed not to change in the future. These internal Chromium values do not provider that guarantee, although I don't know how likely it is that they would change. @david-risney, is that an issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be an issue if the chromium value changed and we changed the value in our public API. We cannot change the value in the public API.

For the names, do we already have enums in our WebView2 APIs similar to this? If so, we should be consistent with those names.

If this is a totally new enum and it needs to exist cross-platform then an OS agnostic naming would be OK. If its new and needs to be different on Win and Mac then it would be good to align the names with the existing OS specific names.

Copy link
Contributor Author

@zhuhaichao518 zhuhaichao518 Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values are new and is sync with upstream and is under cross-platform concept and I expect Mac should have the same enum. Though it is not expected to change frequently, it is possible for them to change the value (it says it is ok to reorganize the value). For this consideration, maybe we remap the constants from them to WebView @bradp0721 @david-risney? We just simply always map COREWEBVIEW2_KEY_EVENT_FLAG_ALT_DOWN to 1 << 3 regardless of how EF_ALT_DOWN changes its value; We map COREWEBVIEW2_KEY_EVENT_FLAG_ALT_DOWN to EF_ALT_DOWN, not its value. It seems CEFSharp is doing like this and append new value at the end when there are new values old version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upstream comment about "It is OK to add values to the middle of this list and/or reorder it" means we have to do an explicit mapping to the COREWEBVIEW2 enum. We cannot just static_cast them and Chromium or Edge could change these values and break our WV2 API.

Instead of having this enum flag, we could also consider adding a GetKeyState function to the event args. The browser side can call GetKeyboardState to get the full keyboard state in a 256 byte array. This would allow the app to query the state of more keys than just the ones we decided to put in the enum. And would make the code the app writes for this event look more similar to the code for AcceleratorKeyPressed. The difference being, one uses ::GetKeyState and one uses event_args->GetKeyState.


/// No additional keys pressed.
COREWEBVIEW2_KEY_EVENT_FLAG_NONE = 0,

/// SHIFT is down, VK_SHIFT.
COREWEBVIEW2_KEY_EVENT_FLAG_SHIFT_DOWN = 1 << 1,

/// Control is down, VK_CONTROL.
COREWEBVIEW2_KEY_EVENT_FLAG_CONTROL_DOWN = 1 << 2,

/// ALT is down, VK_MENU.
/// This bit is 0 when COREWEBVIEW2_KEY_EVENT_FLAG_ALTGR_DOWN bit is 1.
COREWEBVIEW2_KEY_EVENT_FLAG_ALT_DOWN = 1 << 3,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be MENU_DOWN to match VK_MENU?


/// Windows key is down. VK_LWIN | VK_RWIN
COREWEBVIEW2_KEY_EVENT_FLAG_COMMAND_DOWN = 1 << 4,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Command sounds like a Mac term. Should this be WIN_DOWN or WINDOWS_DOWN to match the VK values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine to keep these two enum names. We can use the same enum on Mac and CEFSharp is also using this name (https://cefsharp.github.io/api/113.1.x/html/T_CefSharp_CefEventFlags.htm). (They have not removed 'Mac OS-X command key.' comment though this bit supports windows key now.)


/// Right ALT is down and AltGraph is enabled.
COREWEBVIEW2_KEY_EVENT_FLAG_ALTGR_DOWN = 1 << 6,

/// NUMLOCK is on. VK_NUMLOCK.
COREWEBVIEW2_KEY_EVENT_FLAG_NUM_LOCK_ON = 1 << 8,

/// CapsLock is on. VK_CAPITAL.
COREWEBVIEW2_KEY_EVENT_FLAG_CAPS_LOCK_ON = 1 << 9,

/// ScrollLock is On. VK_SCROLL.
COREWEBVIEW2_KEY_EVENT_FLAG_SCROLL_LOCK_ON = 1 << 10,
} COREWEBVIEW2_KEY_PRESSED_FLAG_KIND;
```

## .NET and WinRT
```c#
namespace Microsoft.Web.WebView2.Core
{
public class CoreWebView2Controller
{
event EventHandler<CoreWebView2UnhandledKeyPressedEventArgs> UnhandledKeyPressed;
}

/// Event args for the `CoreWebView2Controller.UnhandledKeyPressed` event.
runtimeclass CoreWebView2UnhandledKeyPressedEventArgs
{
CoreWebView2KeyEventKind KeyEventKind { get; };

uint VirtualKey { get; };

int KeyEventLParam { get; };

CoreWebView2PhysicalKeyStatus PhysicalKeyStatus { get; };

bool Handled { get; set; };

CoreWebView2KeyPressedFlagKind Modifiers { get; };
}

[Flags] enum CoreWebView2KeyPressedFlagKind
{
CoreWebView2KeyEventFlagNone = 0,
CoreWebView2KeyEventFlagShiftDown = 2,
CoreWebView2KeyEventFlagControlDown = 4,
CoreWebView2KeyEventFlagAltDown = 8,
CoreWebView2KeyEventFlagCommandDown = 16,
CoreWebView2KeyEventFlagAltgrDown = 64,
CoreWebView2KeyEventFlagNumLockOn = 256,
CoreWebView2KeyEventFlagCapsLockOn = 512,
CoreWebView2KeyEventFlagScrollLockOn = 1024,
}
}
```