search

DioxusLabs / dioxus

Fullstack app framework for web, desktop, mobile, and more.
https://dioxuslabs.com
Apache License 2.0
21.45k stars 826 forks source link

Panic in Debug implementation of Event<KeyboardData> #778

Closed MauriceKayser closed 1 year ago

MauriceKayser commented 1 year ago

Problem

could not parse: UnrecognizedKeyError, packages\html\src\events\keyboard.rs:105:38

Stack backtrace: ``` 0: std::panicking::begin_panic_handler at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\panicking.rs:575 1: core::panicking::panic_fmt at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\panicking.rs:64 2: core::result::unwrap_failed at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\result.rs:1790 3: enum2$,keyboard_types::key::UnrecognizedKeyError> >::expect,keyboard_types::key::UnrecognizedKeyError> at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\core\src\result.rs:1069 4: dioxus_html::events::keyboard::KeyboardData::key at dioxus-1e619ce595d3799d\45abbb7\packages\html\src\events\keyboard.rs:105 5: dioxus_html::events::keyboard::impl$1::fmt at dioxus-1e619ce595d3799d\45abbb7\packages\html\src\events\keyboard.rs:152 6: alloc::rc::impl$32::fmt at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\alloc\src\rc.rs:1880 7: core::fmt::builders::impl$3::field::closure$0 at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\fmt\builders.rs:141 8: core::result::Result::and_then at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\result.rs:1371 9: core::fmt::builders::DebugStruct::field at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\fmt\builders.rs:147 10: dioxus_core::events::impl$3::fmt at dioxus-1e619ce595d3799d\45abbb7\packages\core\src\events.rs:102 11: core::fmt::write at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\core\src\fmt\mod.rs:1208 12: std::io::Write::write_fmt at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\io\mod.rs:1682 13: std::io::stdio::impl$12::write_fmt at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\io\stdio.rs:715 14: std::io::stdio::impl$11::write_fmt at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\io\stdio.rs:689 15: std::io::stdio::print_to at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\io\stdio.rs:1007 16: std::io::stdio::_print at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9/library\std\src\io\stdio.rs:1074 17: dioxus_problem::APP::closure$0::closure$0::closure$0 at .\src\main.rs:9 18: dioxus_core::scopes::impl$1::listener::closure$0 at dioxus-1e619ce595d3799d\45abbb7\packages\core\src\scopes.rs:501 19: dioxus_core::virtual_dom::VirtualDom::handle_event at dioxus-1e619ce595d3799d\45abbb7\packages\core\src\virtual_dom.rs:422 20: dioxus_desktop::launch_with_props::closure$0 > at dioxus-1e619ce595d3799d\45abbb7\packages\desktop\src\lib.rs:193 21: tao::platform_impl::platform::event_loop::impl$2::run_return::closure$0 > > at tao-0.15.8\src\platform_impl\windows\event_loop.rs:231 22: alloc::boxed::impl$46::call_mut >,ref_mut$ > >,dyn$ at tao-0.15.8\src\platform_impl\windows\event_loop\runner.rs:250 24: core::panic::unwind_safe::impl$23::call_once,tao::platform_impl::platform::event_loop::runner::impl$3::call_event_handler::closure_env$0 > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\core\src\panic\unwind_safe.rs:271 25: std::panicking::try::do_call >,tuple$<> > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panicking.rs:483 26: std::sync::rwlock::impl$19::deref_mut > > 27: std::panicking::try,core::panic::unwind_safe::AssertUnwindSafe > > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panicking.rs:447 28: std::panic::catch_unwind >,tuple$<> > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panic.rs:140 29: tao::platform_impl::platform::event_loop::runner::EventLoopRunner::catch_unwind,tao::platform_impl::platform::event_loop::runner::impl$3::call_even at tao-0.15.8\src\platform_impl\windows\event_loop\runner.rs:156 30: tao::platform_impl::platform::event_loop::runner::EventLoopRunner::call_event_handler at tao-0.15.8\src\platform_impl\windows\event_loop\runner.rs:242 31: tao::platform_impl::platform::event_loop::runner::EventLoopRunner::send_event at tao-0.15.8\src\platform_impl\windows\event_loop\runner.rs:224 32: tao::platform_impl::platform::event_loop::ThreadMsgTargetSubclassInput::send_event at tao-0.15.8\src\platform_impl\windows\event_loop.rs:124 33: tao::platform_impl::platform::event_loop::thread_event_target_callback::closure$0 at tao-0.15.8\src\platform_impl\windows\event_loop.rs:2195 34: core::ops::function::FnOnce::call_once,tuple$<> > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\core\src\ops\function.rs:250 35: core::panic::unwind_safe::impl$23::call_once > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\core\src\panic\unwind_safe.rs:271 36: std::panicking::try::do_call >,windows::Windows::Win32::Foundation::LRESULT> at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panicking.rs:483 37: std::sync::rwlock::impl$19::deref_mut > > 38: std::panicking::try > > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panicking.rs:447 39: std::panic::catch_unwind >,windows::Windows::Win32::Foundation::LRESULT> at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\std\src\panic.rs:140 40: tao::platform_impl::platform::event_loop::runner::EventLoopRunner::catch_unwind at tao-0.15.8\src\platform_impl\windows\event_loop.rs:2244 42: DefSubclassProc 43: DefSubclassProc 44: CallWindowProcW 45: DispatchMessageW 46: windows::Windows::Win32::UI::WindowsAndMessaging::DispatchMessageW at windows-0.39.0\src\Windows\Win32\UI\WindowsAndMessaging\mod.rs:2671 47: tao::platform_impl::platform::event_loop::EventLoop::run_return > > at tao-0.15.8\src\platform_impl\windows\event_loop.rs:261 48: tao::platform_impl::platform::event_loop::EventLoop::run > > at tao-0.15.8\src\platform_impl\windows\event_loop.rs:215 49: tao::event_loop::EventLoop::run > > at tao-0.15.8\src\event_loop.rs:179 50: dioxus_desktop::launch_with_props > at dioxus-1e619ce595d3799d\45abbb7\packages\desktop\src\lib.rs:137 51: dioxus_desktop::launch at dioxus-1e619ce595d3799d\45abbb7\packages\desktop\src\lib.rs:58 52: dioxus_problem::main at .\src\main.rs:4 53: core::ops::function::FnOnce::call_once > at /rustc/3020239de947ec52677e9b4e853a6a9fc073d1f9\library\core\src\ops\function.rs:250 ```

Steps To Reproduce

  1. Add code:
use dioxus::prelude::*;

fn main() {
    dioxus_desktop::launch(APP);
}

static APP: Component = |cx| {
    cx.render(rsx!(input {
        onkeypress: move |e| { println!("{:?}", e) }
    }))
};
  1. Start the app, focus the input, hold Ctrl, press A.

Environment:

Questionnaire

ealmloff commented 1 year ago

By logging the key (here) in the interpreter, you can see that the webview outputs unicode escape characters instead of the key pressed: Ctrl-S outputs key: "\u0013", Ctrl-A outputs key: "\u0001". Which fails to deserialize. (Note these unicode characters are control characters, not the codes for s and a) Here is the same example in JS: https://jsfiddle.net/6uxko9a2/

MauriceKayser commented 1 year ago

For reference of the Ctrl+@ - Ctrl+_ keys: https://www.physics.udel.edu/~watson/scen103/ascii.html

I pressed Ctrl+A in your jsfiddle in a few different browsers: Brave (Chromium), Edge (Chromium) and Firefox, and recorded the relevant values of the KeyboardEvent:

OS Browser Chromium charCode code ctrlKey key keyCode which
Linux Brave Yes 1 "KeyA" true "a" 1 1
Linux Edge Yes 1 "KeyA" true "a" 1 1
Windows Brave Yes 1 "KeyA" true "\u0001" 1 1
Windows Edge Yes 1 "KeyA" true "\u0001" 1 1
Linux & Windows Firefox No - - - - - -

Seems like the issue is related to Windows + Chromium. Firefox does not generate any KeyboardEvent for the Ctrl+ key presses. charCode, keyCode and which all seem to be the same (e.g. when trying with Ctrl+B they all are 2). I could only get Chromium to output the control codes for Ctrl+A - Ctrl+Z on my German keyboard, not any of the Ctrl+@, Ctrl+[ etc.

Which option would you prefer, adding the Ctrl+[ to Ctrl+_ (or only Ctrl+A - Ctrl+Z) keys to the keyboard-types crate, or modifing dioxus, for example by patching key from "\u0001" to "a"?

ealmloff commented 1 year ago

If we override this behavior it should probably be at the Dioxus level, not in keyboard-types. Decoding the Unicode values for Escape and Backspace as not escape and backspace should probably not be part of that crate.

Reading the mdn docs on KeyboardEvent.key, the escape and backspace key codes are a valid value for key. If they are it feels wrong to decode the Unicode version of escape as something other than escape or backspace.

React's solution seems to be to just ignore the event in all cases: https://playcode.io/1170117

We should definitely not panic when we fail to decode the character. We can use the Unknown variant of the enum instead.

This issue also appears to be specific to onkeypress, onkeydown works fine in the js playground example