Skip to content
This repository has been archived by the owner on Feb 17, 2022. It is now read-only.

Keyboard input does not handle DOM keyboard events with modern attributes #108

Closed
sy2002 opened this issue Mar 18, 2020 · 10 comments
Closed

Comments

@sy2002
Copy link

sy2002 commented Mar 18, 2020

This issue is about a discussion I had with the maintainers of Firefox:

https://bugzilla.mozilla.org/show_bug.cgi?id=1622670

There is a bug/problem in Firefox, which leads to the effect, that this very simple SDL program is not able to receive keypresses like "+" and "#" on german keyboards at the Emscripten/WebAssembly/WebGL target. Here is the code:

https://pastebin.com/k6b153mf

Here is this code, compiled with Emscripten and SDL for Emscripten to play with online:

http://sy2002.de/keytest/keytest.html

Open the website, type on your keyboard and see, how you see the scancode and the keycode in the console. Now, if you use Chrome and Safari: No problems at all. But if you use Firefox and a German keyboard, then you are not able to enter a "+" or a "#" or a "^" or a "ß" because as you can see in the demo above: There are zero events forwarded to SDL by Firefox.

The Firefox team claims (citation from the bugzilla link above): "I guess it [SDL] does not handle DOM keyboard events with modern attributes."

After that, the Firefox team lowered the priority to P3, which means it might take quite long to be fixed.

I do not know what the Chrome team or the Safari team do different - because everthing works fine there - and I do not know if they do it right or Firefox does it right.

Unfortunatelly, I am not savvy enough in DOM to have an own opinion. This is why I wanted to share this with the Emscripten/SDL2 community: Just in case the Firefox team is right and the SDL port is really (whatever this means) "not handling DOM keyboard events with modern attributes", then this might be a bug in the SDL port.

@Daft-Freak
Copy link
Member

Yes, we are using the legacy keyCode for keyboard events. This is mostly because it was better supported at the time and also because the modern attributes are strings which makes the translation a bit harder than an array lookup.

It might be possible to fix this without a rewrite depending on the keyCode values for those keys. (I'm surprised there haven't been any issues about this already since I've only tested on English keyboards).

Relevant code:
https://github.com/emscripten-ports/SDL2/blob/master/src/video/emscripten/SDL_emscriptenevents.c#L45
https://github.com/emscripten-ports/SDL2/blob/master/src/video/emscripten/SDL_emscriptenevents.c#L503

@sy2002
Copy link
Author

sy2002 commented Mar 23, 2020

@Daft-Freak Thank you for your feedback. Do you have an idea, why Emscripten's SDL works perfectly fine on Safari and Chrome but not on Firefox?

About your feedback:

I went to https://keycode.info/ and found out that for example the keycode for the non working "+" key on the German keyboard is 171.

So this

    /* 171 */   SDL_SCANCODE_UNKNOWN,

here
https://github.com/emscripten-ports/SDL2/blob/master/src/video/emscripten/SDL_emscriptenevents.c#L217

would need to be changed to SDL_SCANCODE_PLUS.

Unfortunatelly, there is no SDL_SCANCODE_PLUS but just a SDL_SCANCODE_MINUS:

https://wiki.libsdl.org/SDL_Scancode

Might sending unknown Scancodes via SDL_SendKeyboardKey(eventType == EMSCRIPTEN_EVENT_KEYDOWN ? SDL_PRESSED : SDL_RELEASED, scancode); solve the issue, because then the app can descide how to proceed with unknown codes? (currently, if I read your code correctly, unknown scancodes are not sent)

@sy2002
Copy link
Author

sy2002 commented Mar 23, 2020

@Daft-Freak : Additionally to what I wrote above:

I had another idea: Firefox does not work. But Chrome does. So I used Chrome, went to http://sy2002.de/keytest/keytest.html and pressed "+":

It outputs "Scancode: 46" and "Keycode: 61".

According to https://wiki.libsdl.org/SDLScancodeLookup scancode 46 is SDL_SCANCODE_EQUALS

Would this mean, that

https://github.com/emscripten-ports/SDL2/blob/master/src/video/emscripten/SDL_emscriptenevents.c#L217

would need to read

/* 171 */   SDL_SCANCODE_EQUALS,

If this is the right approach, here are the values - there are just 5 keys that are not working:

/*  60  */   SDL_SCANCODE_COMMA,
/*  63  */   SDL_SCANCODE_MINUS,
/* 160 */   SDL_SCANCODE_GRAVE,
/* 163 */   SDL_SCANCODE_BACKSLASH,
/* 171 */    SDL_SCANCODE_EQUALS,

EDIT: No that does not seem to be the right approach, because e.g. pressing comma "," leads to the same keycode in Chrome than pressing "<". I guess I will need to experiment a bit using Emscripten and a locally compiled version of SDL2.

@Daft-Freak
Copy link
Member

Hmm, I would check what scancodes native SDL generates. If some browsers are generating the same keyCode for multiple keys, we may need to use the code attribute for at least some keys.

@sy2002
Copy link
Author

sy2002 commented Mar 25, 2020

@Daft-Freak You are right, using native SDL helped: I did compile the above-mentioned test program natively. Here are the results:

Key DOM KeyCodes SDL native mapping:
< /* 60 */ 100 => SDL_SCANCODE_NONUSBACKSLASH
ß /* 63 */ 45 => SDL_SCANCODE_MINUS
^ /* 160 */ 53 => SDL_SCANCODE_GRAVE
# /* 163 */ 49 => SDL_SCANCODE_BACKSLASH
+ /* 171 */ 48 => SDL_SCANCODE_RIGHTBRACKET

Additionally, as a sanity check: I checked the positions of e.g. SDL_SCANCODE_MINUS on a U.S. keyboard and compared them to e.g. the position of the "ß" key on a German keyboard: Fits nicely. so the above-mentioned mapping seems to be sensible.

If you'd like to add it like this, would be great. I can do a PR, if you prefer.

@Daft-Freak
Copy link
Member

I've pushed a commit with those mappings (since it shouldn't break anything).

@sy2002
Copy link
Author

sy2002 commented Mar 25, 2020

Thank you for your fast help! :-) I will gladly test it so that we can close this issue, soon.

@sy2002
Copy link
Author

sy2002 commented Mar 26, 2020

@Daft-Freak Thank you that you committed this one so quickly. I just wanted to test it and need to admit that I never compiled SDL2 for emscripten before. I conveniently always used -s USE_SDL=2. So here is what I did and what I got:

  1. Compiled using cmake and this instruction:
    https://github.com/emscripten-ports/SDL2/blob/master/docs/README-emscripten.md
$ mkdir build
$ cd build
$ emcmake cmake ..
$ emmake make

Worked. I got libSDL2.a and libSDL2main.a.

  1. Compiled the simple keyboard test program (link) using this:
emcc keytest.c SDL2/build/libSDL2.a -o keytest.html -s ASYNCIFY

Worked. I got keytest.html keytest.wasm keytest.js

  1. Opened it in my browser and here is the result:
01:12:49.222
exception thrown: InternalError: too much recursion,@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[1164]:0x298af1
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[360]:0x3281f
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[1028]:0x25c25c
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[258]:0xcd0e
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[261]:0xd3d6
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[360]:0x3286a
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[1028]:0x25c25c
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[258]:0xcd0e
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:wasm-function[261]:0xd3d6
@http://localhost:8080/keytest.js line 1769 > WebAssembly.instantiate:was…

Obviously I did something wrong :-) Any ideas?

@Daft-Freak
Copy link
Member

Hmm, it's been a while since I've compiled manually. Possibly a conflict with SDL1? You should be able to use EMCC_LOCAL_PORTS="sdl2=/path/to/sdl/source" to get -s USE_SDL=2 to use your local version.

@sy2002
Copy link
Author

sy2002 commented Mar 26, 2020

@Daft-Freak Thank you this was very helpful: It worked, I was able to test and we can close the issue now:

  • It works as expected in Firefox now.

  • In Chrome and Safari it works the same way as it worked before the patch (as mentioned above: which is kind of wrong).

I think on the long run, we might need to switch to the modern attributes, but I guess for now everything is OK.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants