Fix WindowEvent::ReceivedCharacter on web

[tronical]

The event was never sent to the application because of the unconditional
preventDefault()
call on keydown.

Fixes #1741

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[ryanisaacg]

Looks good! Before we merge, we have to address the compile error on stdweb. Importing stdweb::web::event::IKeyboardEvent should do the trick.

[tronical]

Thank you! That did the trick :)

[alvinhochun]

See inline review. Also, I think the code deserves at least a short comment to describe what it does and why it is needed.

[alvinhochun]

Shouldn't this be put inside on_keyboard_press instead of on_keyboard_release?

[tronical]

Ouch, yes, of course. Thanks for spotting!

[alvinhochun]

Sorry, this doesn't block normal shortcut keys (e.g. Ctrl+R), but it does block function keys (e.g. F5). It does appear that we still need to check for modifiers. This should work, but I'm not entirely sure if it does what people usually expects AltGr to do:

let is_shortcut_modifiers = (event.ctrl_key() || event.alt_key()) && !event.get_modifier_state("AltGr");
if !is_key_string || is_shortcut_modifiers {
    event.prevent_default();
}

Also, I believe you need to add preventDefault to the keypress listener to stop Space or Shift+Space from scrolling the page.

[tronical]

Sorry, this doesn't block normal shortcut keys (e.g. Ctrl+R), but it does block function keys (e.g. F5). It does appear that we still need to check for modifiers. This should work, but I'm not entirely sure if it does what people usually expects AltGr to do:

let is_shortcut_modifiers = (event.ctrl_key() || event.alt_key()) && !event.get_modifier_state("AltGr");
if !is_key_string || is_shortcut_modifiers {
event.prevent_default();
}

Done. Also added the missing comment, you're totally right that the handling needs it.

Also, I believe you need to add preventDefault to the keypress listener to stop Space or Shift+Space from scrolling the page.

I'm not sure I understand, can you elaborate?

[tronical]

The remaining CI failure looks like an unrelated network problem to me.

[alvinhochun]

Also, I believe you need to add preventDefault to the keypress listener to stop Space or Shift+Space from scrolling the page.

I'm not sure I understand, can you elaborate?

Open https://codepen.io/alvinhochun/pen/yLJpwGX, focus on the canvas and try pressing Space. You will find that the page gets scrolled down. Uncomment e.preventDefault(); in the keypress listener and you will find that this behaviour is blocked.

[tronical]

Open https://codepen.io/alvinhochun/pen/yLJpwGX, focus on the canvas and try pressing Space. You will find that the page gets scrolled down. Uncomment e.preventDefault(); in the keypress listener and you will find that this behaviour is blocked.

Got it, thanks!

Added this in a follow-up commit on top (and fixed changelog grammar in initial commit).

[alvinhochun]

Looks good to me!