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

wasm: WindowEvent::CharacterReceived is not sent anymore #1741

Closed
tronical opened this issue Oct 13, 2020 · 10 comments · Fixed by #1747
Closed

wasm: WindowEvent::CharacterReceived is not sent anymore #1741

tronical opened this issue Oct 13, 2020 · 10 comments · Fixed by #1747
Labels

Comments

@tronical
Copy link
Contributor

Since 0.23 WindowEvent::CharacterReceived is not sent anymore. I tracked it down to commit 6cfddfe at

event.prevent_default();
where the prevent_default() call results in the keypress event to be never received on the canvas element.

This is happening at least with Safari and Chrome on macOS.

@kchibisov
Copy link
Member

Please, next time provide platform you're using more clearly, so it'll save us time guessing about what you're talking about.

cc @ryanisaacg

@tronical
Copy link
Contributor Author

Apologies. What would be the best/preferred way to reduce the amount of guessing for you? (for future reports)

@kchibisov
Copy link
Member

Ah, you've put wasm in the title, I forgot to look into it, since I was looking into platform: something in a body, I guess it's fine, but there're 2 web platforms iirc(one is deprecated though).

something like

platform: web_sys

Could help.

@alvinhochun
Copy link
Contributor

alvinhochun commented Oct 13, 2020

That commit for PR #1576 was to fix #1573. That issue focused on page scrolling, but scrolling is not the only concern. Here are some examples of keys that associates with some browser default actions (excluding the unblockable ones):

  • Tab or Shift+Tab focus switching
  • Arrow keys, PgUp, PgDn, Home, End and Space/Shift+Space to scroll the page
  • Backspace (Firefox) or Alt+Left/Right history navigation
  • Ctrl+L or Alt+D to focus address bar
  • F5 or Ctrl+R to reload page
  • F11 for fullscreen (Winit users might want to make its Winit window fullscreen instead)
  • F12 for dev tools

It seems reasonable that Winit should block their default behaviour when the canvas is in focus (except for Tab/Shift+Tab (*) ).

We may change Winit to only call preventDefault on keydown for keys that cannot generate character input (taking into account the modifier keys), so that it won't stop keypress from being generated. In addition to this, preventDefault will also need to be called in the keypress event to block the Space key scrolling.

(*) Tab/Shift+Tab is a bit tricky due to accessibility. Blocking them is fine for games, but some use cases might benefit from conditionally blocking/unblocking them. However, accessibility for canvas elements is pretty bad in general, so we can probably just ignore this for now.

@tronical
Copy link
Contributor Author

We may change Winit to only call preventDefault on keydown for keys that cannot generate character input (taking into account the modifier keys), so that it won't stop keypress from being generated.

That makes sense. I see why it's necessary to call preventDefault().

The MDN docs suggest that the key property should be a "non-empty Unicode character string" or one of the pre-defined values if the pressed key is a control or special character. Perhaps it would be sufficient to compare the key against the pre-defined values and if it's one of those, then call preventDefault?

I'd be happy to try to make a patch, if there's agreement on the approach.

@ryanisaacg
Copy link
Contributor

I think the approach of "see if this key can be typed, and if so do not prevent default" is a good one.

@alvinhochun
Copy link
Contributor

I think matching against a list of keys that can produce a character input should be good. The list would probably be a combination of alphanumeric keys and some symbol keys. Note that it also needs to check the modifier keys so that it can also block some key combinations like Ctrl+L, without preventing Shift+L from producing uppercase "L".

I am a bit worried about accented characters that are input by AltGr+alpha as I don't know how they work at all. It might be a good idea to write a small JavaScript POC to test for a bit on different platforms and browsers.

IME composition is another beast which might have an effect, but since Winit doesn't yet support IME composition we don't need to deal with it now.

@tronical
Copy link
Contributor Author

Wouldn't it perhaps be easier to go the other way around and instead of checking whether the key can be typed, check whether the key can not be typed? It seems to me that this would be a shorter and more well-defined (specified) set.

In particular I'm thinking of what's listed here https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values as well as "Dead" and "Undefined".

I might very well be missing something though :)

@alvinhochun
Copy link
Contributor

alvinhochun commented Oct 15, 2020

I took another look and I think checking whether the key can be typed would be easier. The simple way might be to just check whether KeyboardEvent.key contains a valid key string (note that it is UTF-16 in JS but web-sys might have converted it to a UTF-8 Rust String). I don't know if there are any traps with this method though.

We might also be able to cheat by just doing

let event_key: &str = event.key();
let is_key_string = event_key.len() == 1 || !event_key.is_ascii();

This would be more generic than comparing against a predefined list.

See https://www.w3.org/TR/uievents-key/#key-attr-values, and I quote:

A key string is a string containing a 0 or 1 non-control characters ("base" characters) followed by 0 or more combining characters. The string MUST be in Normalized Form C (NFC) as described in [UnicodeNormalizationForms].

tronical added a commit to tronical/winit that referenced this issue Oct 27, 2020
The event was never sent to the application because of the unconditional
preventDefault()
call on keydown.

Fixes rust-windowing#1741
@tronical
Copy link
Contributor Author

I've implemented the logic suggested by @alvinhochun in the linked PR.

tronical added a commit to tronical/winit that referenced this issue Oct 27, 2020
The event was never sent to the application because of the unconditional
preventDefault()
call on keydown.

Fixes rust-windowing#1741
tronical added a commit to tronical/winit that referenced this issue Oct 28, 2020
The event was never sent to the application because of the unconditional
preventDefault() call on keydown.

Fixes rust-windowing#1741
tronical added a commit to tronical/winit that referenced this issue Oct 28, 2020
The event was never sent to the application because of the unconditional
preventDefault() call on keydown.

Fixes rust-windowing#1741
tronical added a commit to tronical/winit that referenced this issue Oct 28, 2020
The event was never sent to the application because of the unconditional
preventDefault() call on keydown.

Fixes rust-windowing#1741
tronical added a commit to tronical/winit that referenced this issue Oct 28, 2020
After reaching keypress, we should prevent further propagation.

Relates to rust-windowing#1741
ryanisaacg pushed a commit that referenced this issue Oct 29, 2020
* Fix WindowEvent::ReceivedCharacter on web

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

Fixes #1741

* Don't scroll when pressing space on a focused canvas

After reaching keypress, we should prevent further propagation.

Relates to #1741
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants