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

Keyboard.presses emitting '0' for most keys in Firefox #326

Closed
brendanzab opened this Issue Aug 3, 2015 · 14 comments

Comments

Projects
None yet
7 participants
@brendanzab

brendanzab commented Aug 3, 2015

I am using Firefox 41.0a2 (2015-08-02)

import Graphics.Element exposing (..)
import Keyboard


main : Signal Element
main =
  Signal.map show Keyboard.presses

The arrow keys emit the right key codes, but most of the other keys emit 0. It works fine on Chrome.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 3, 2015

Contributor

I can reproduce the Firefox part. On Chrome, I actually see the "other keys" working, but not the arrow keys.

Contributor

jvoigtlaender commented Aug 3, 2015

I can reproduce the Firefox part. On Chrome, I actually see the "other keys" working, but not the arrow keys.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 3, 2015

Contributor

Probably Keyboard.presses should be deprecated. It does not do what its current documentation suggests. See https://developer.mozilla.org/en-US/docs/Web/Events/keypress, which says: "... is fired when a key is pressed down and that key normally produces a character value". The current documentation of Keyboard.presses does in no way convey the "[if] that key normally produces a character value" part.

Moreover, the current implementation of Keyboard.presses uses the keyCode attribute, on which MDN has to say: "Warning: This attribute is deprecated; you should use key instead, if available."

Contributor

jvoigtlaender commented Aug 3, 2015

Probably Keyboard.presses should be deprecated. It does not do what its current documentation suggests. See https://developer.mozilla.org/en-US/docs/Web/Events/keypress, which says: "... is fired when a key is pressed down and that key normally produces a character value". The current documentation of Keyboard.presses does in no way convey the "[if] that key normally produces a character value" part.

Moreover, the current implementation of Keyboard.presses uses the keyCode attribute, on which MDN has to say: "Warning: This attribute is deprecated; you should use key instead, if available."

@brendanzab

This comment has been minimized.

Show comment
Hide comment
@brendanzab

brendanzab Aug 3, 2015

I decided to go with:

input : Signal Action
input =
  let onKeysDown codes value =
        let allKeys keys =
              List.all (\x -> Set.member x keys) codes
            valueOrNoOp keys =
              if | allKeys keys -> value
                 | otherwise -> NoOp
        in
          Keyboard.keysDown
            |> Signal.map valueOrNoOp
            |> Signal.dropRepeats

      hashToAction hash =
        parseHash hash
          |> Maybe.map (SlideShow.goto >> SlideShow)
          |> Maybe.withDefault NoOp
  in
    Signal.mergeMany
      [ Signal.map hashToAction (Signal.dropRepeats History.hash)
      , onKeysDown [ 16, 32 ] (SlideShow SlideShow.previous) -- shift + space
      , onKeysDown [ 32 ] (SlideShow SlideShow.next) -- space
      , onKeysDown [ 37 ] (SlideShow SlideShow.previous) -- left arrow
      , onKeysDown [ 39 ] (SlideShow SlideShow.next) -- right arrow
      , onKeysDown [ 80 ] (SlideShow SlideShow.toggleMode) -- p
      , actions.signal
      ]

Kinda ugly... :/

brendanzab commented Aug 3, 2015

I decided to go with:

input : Signal Action
input =
  let onKeysDown codes value =
        let allKeys keys =
              List.all (\x -> Set.member x keys) codes
            valueOrNoOp keys =
              if | allKeys keys -> value
                 | otherwise -> NoOp
        in
          Keyboard.keysDown
            |> Signal.map valueOrNoOp
            |> Signal.dropRepeats

      hashToAction hash =
        parseHash hash
          |> Maybe.map (SlideShow.goto >> SlideShow)
          |> Maybe.withDefault NoOp
  in
    Signal.mergeMany
      [ Signal.map hashToAction (Signal.dropRepeats History.hash)
      , onKeysDown [ 16, 32 ] (SlideShow SlideShow.previous) -- shift + space
      , onKeysDown [ 32 ] (SlideShow SlideShow.next) -- space
      , onKeysDown [ 37 ] (SlideShow SlideShow.previous) -- left arrow
      , onKeysDown [ 39 ] (SlideShow SlideShow.next) -- right arrow
      , onKeysDown [ 80 ] (SlideShow SlideShow.toggleMode) -- p
      , actions.signal
      ]

Kinda ugly... :/

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 3, 2015

Contributor

Oh, yeah. My suggestion that Keyboard.presses be deprecated came from the assumption that uses of it can be replaced by other functions from the Keyboard module. What you have written now seems to contradict that. :-)

Though I do wonder whether it shouldn't be possible to retrieve what you want from Keyboard.keysDown by set differencing somehow. That is, a foldp over it that always compares the current set of down-keys to the previous such set.

But in any case, let's assume Keyboard.presses is not deprecated. Then, I think:

  • Its documentation should change, since it currently says something else than what is indicated by https://developer.mozilla.org/en-US/docs/Web/Events/keypress
  • We have to face the fact that only Chrome currently does what it is supposed to do regarding this event (namely giving the character code for "normal keys" like letters etc).
  • Maybe we also have to do something about the warning from MDN I quoted above: "Warning: This attribute is deprecated; you should use key instead, if available."

Finally, maybe worth taking a look at http://javascript.info/tutorial/keyboard-events, which seems to shed some light on the oddities of keypress vs. keydown.

Contributor

jvoigtlaender commented Aug 3, 2015

Oh, yeah. My suggestion that Keyboard.presses be deprecated came from the assumption that uses of it can be replaced by other functions from the Keyboard module. What you have written now seems to contradict that. :-)

Though I do wonder whether it shouldn't be possible to retrieve what you want from Keyboard.keysDown by set differencing somehow. That is, a foldp over it that always compares the current set of down-keys to the previous such set.

But in any case, let's assume Keyboard.presses is not deprecated. Then, I think:

  • Its documentation should change, since it currently says something else than what is indicated by https://developer.mozilla.org/en-US/docs/Web/Events/keypress
  • We have to face the fact that only Chrome currently does what it is supposed to do regarding this event (namely giving the character code for "normal keys" like letters etc).
  • Maybe we also have to do something about the warning from MDN I quoted above: "Warning: This attribute is deprecated; you should use key instead, if available."

Finally, maybe worth taking a look at http://javascript.info/tutorial/keyboard-events, which seems to shed some light on the oddities of keypress vs. keydown.

@sindikat

This comment has been minimized.

Show comment
Hide comment
@sindikat

sindikat Aug 10, 2015

We definitely need something that simply returns the last pressed key, regardless of whether it is pressed right now or not. For example, I am trying to write a roguelike, and using Keyboard.arrows is inconvenient. I'll explain:

  • You press UP, and Signal emits {x=0, y=1}
  • The model is updated accordingly
  • You press RIGHT but haven't unpressed UP yet, so Signal emits {x=1, y=1}
  • The model updates x, but it also updates y again
  • Therefore there is weird behavior when trying to walk diagonally

Try to move diagonally (quickly press up, left, up, left, up, left, or something like that):

http://share-elm.com/sprout/55c89e54e4b06aacf0e8bea6

I have no idea how to implement correct movement in my roguelike, so that I don't need to fiddle with “is this still pressed or not”. Keyboard.presses doesn't work in my Firefox 39.0.

sindikat commented Aug 10, 2015

We definitely need something that simply returns the last pressed key, regardless of whether it is pressed right now or not. For example, I am trying to write a roguelike, and using Keyboard.arrows is inconvenient. I'll explain:

  • You press UP, and Signal emits {x=0, y=1}
  • The model is updated accordingly
  • You press RIGHT but haven't unpressed UP yet, so Signal emits {x=1, y=1}
  • The model updates x, but it also updates y again
  • Therefore there is weird behavior when trying to walk diagonally

Try to move diagonally (quickly press up, left, up, left, up, left, or something like that):

http://share-elm.com/sprout/55c89e54e4b06aacf0e8bea6

I have no idea how to implement correct movement in my roguelike, so that I don't need to fiddle with “is this still pressed or not”. Keyboard.presses doesn't work in my Firefox 39.0.

@sindikat

This comment has been minimized.

Show comment
Hide comment
@sindikat

sindikat Aug 10, 2015

I think I implemented a workaround. Using Signal.foldp I implemented a Signal that emits the last pressed key, by maintaining internal state. I have 2 variants, Maybe KeyCode and KeyCode, with slightly different behavior:

(Minor note: in the first link note how in Share-Elm the pressed key quickly becomes Nothing after a second, while in Try-Elm or in real Elm it stays Just something until you lift the key)

Here are examples of integrating with my roguelike:

Try to move the @ character by pressing arrows. Note how it works correctly in the first example, but in the second example there is some weird movements going on. It's because the internal state is updated every time a key is pressed or lifted, so even if the actualKey stays the same, it is now called twice.

Therefore I recommend using the version of a workaround that uses Maybe KeyCode instead of KeyCode.

sindikat commented Aug 10, 2015

I think I implemented a workaround. Using Signal.foldp I implemented a Signal that emits the last pressed key, by maintaining internal state. I have 2 variants, Maybe KeyCode and KeyCode, with slightly different behavior:

(Minor note: in the first link note how in Share-Elm the pressed key quickly becomes Nothing after a second, while in Try-Elm or in real Elm it stays Just something until you lift the key)

Here are examples of integrating with my roguelike:

Try to move the @ character by pressing arrows. Note how it works correctly in the first example, but in the second example there is some weird movements going on. It's because the internal state is updated every time a key is pressed or lifted, so even if the actualKey stays the same, it is now called twice.

Therefore I recommend using the version of a workaround that uses Maybe KeyCode instead of KeyCode.

@bburdette

This comment has been minimized.

Show comment
Hide comment
@bburdette

bburdette Sep 26, 2015

ok in core/keyboard.js there's this function here:

function keyStream(node, eventName, handler)
{
    var stream = NS.input(eventName, { _: {}, alt: false, meta: false, keyCode: 0 });
    localRuntime.addListener([stream.id], node, eventName, function(e) {
        localRuntime.notify(stream.id, handler(e));
    });
    return stream;
}

And if you inspect 'e' in this function, in firefox you have e.key = "d" (for instance) and e.keycode = 0. In chromium its e.keycode = 100 (or whatever) and there is no e.key at all.

So then keyStream is used here:

var downs = keyStream(document, 'keydown', keyEvent);
var ups = keyStream(document, 'keyup', keyEvent);
var presses = keyStream(document, 'keypress', keyEvent);
var blurs = keyStream(window, 'blur', function() { return null; });

And this function keyEvent is passed in, which is this:

function keyEvent(event)
{
    return {
        _: {},
        alt: event.altKey,
        meta: event.metaKey,
        keyCode: event.keyCode
    };
}

So I guess what needs to happen is to convert e.key to a keyCode in this function, if it exists. Otherwise use the e.keyCode. Not sure how this would play out in safari or other browsers though.

bburdette commented Sep 26, 2015

ok in core/keyboard.js there's this function here:

function keyStream(node, eventName, handler)
{
    var stream = NS.input(eventName, { _: {}, alt: false, meta: false, keyCode: 0 });
    localRuntime.addListener([stream.id], node, eventName, function(e) {
        localRuntime.notify(stream.id, handler(e));
    });
    return stream;
}

And if you inspect 'e' in this function, in firefox you have e.key = "d" (for instance) and e.keycode = 0. In chromium its e.keycode = 100 (or whatever) and there is no e.key at all.

So then keyStream is used here:

var downs = keyStream(document, 'keydown', keyEvent);
var ups = keyStream(document, 'keyup', keyEvent);
var presses = keyStream(document, 'keypress', keyEvent);
var blurs = keyStream(window, 'blur', function() { return null; });

And this function keyEvent is passed in, which is this:

function keyEvent(event)
{
    return {
        _: {},
        alt: event.altKey,
        meta: event.metaKey,
        keyCode: event.keyCode
    };
}

So I guess what needs to happen is to convert e.key to a keyCode in this function, if it exists. Otherwise use the e.keyCode. Not sure how this would play out in safari or other browsers though.

@esad

This comment has been minimized.

Show comment
Hide comment
@esad

esad Nov 16, 2015

Looking at how jQuery normalizes key codes, using something like

keyCode: (event.which != null) ? event.which : (event.charCode != null ? event.charCode : event.keyCode)

in Native/Keyboard.js should do it.

esad commented Nov 16, 2015

Looking at how jQuery normalizes key codes, using something like

keyCode: (event.which != null) ? event.which : (event.charCode != null ? event.charCode : event.keyCode)

in Native/Keyboard.js should do it.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Nov 17, 2015

Contributor

Yes, this looks good. Do you want to make a pull request for it?

@bjz, @sindikat: In the demo box at https://api.jquery.com/event.which/ you should be able to test whether this polyfill will give the desired behavior (meaningful codes for arrow keys as well as for normal character keys) on the browsers you were using and experiencing problems with before. Maybe you can report back if things work for you there?

Contributor

jvoigtlaender commented Nov 17, 2015

Yes, this looks good. Do you want to make a pull request for it?

@bjz, @sindikat: In the demo box at https://api.jquery.com/event.which/ you should be able to test whether this polyfill will give the desired behavior (meaningful codes for arrow keys as well as for normal character keys) on the browsers you were using and experiencing problems with before. Maybe you can report back if things work for you there?

@esad

This comment has been minimized.

Show comment
Hide comment
@esad

esad Nov 17, 2015

@jvoigtlaender I've made the change in my local fork and it's working fine for me - I was bit unsure how to do the tests for this. I'll submit the PR so maybe we can discuss it there.

esad commented Nov 17, 2015

@jvoigtlaender I've made the change in my local fork and it's working fine for me - I was bit unsure how to do the tests for this. I'll submit the PR so maybe we can discuss it there.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Nov 17, 2015

Contributor

@bjz, @sindikat: If you want to follow through on my suggestion to test whether this will fix the problems for you in your environments, you should actually use http://javascript.info/tutorial/keyboard-events instead of https://api.jquery.com/event.which/ that I suggested earlier. Because the latter tests keydown, not keypress.

Contributor

jvoigtlaender commented Nov 17, 2015

@bjz, @sindikat: If you want to follow through on my suggestion to test whether this will fix the problems for you in your environments, you should actually use http://javascript.info/tutorial/keyboard-events instead of https://api.jquery.com/event.which/ that I suggested earlier. Because the latter tests keydown, not keypress.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 7, 2015

Member

I sorted out a fix that should definitely preserve the existing behavior independent of how crazy browsers are. Hopefully it looks okay!

I have a few other things I'm working on in core, so I'll try to do a patch or minor version in the next week or so.

Member

evancz commented Dec 7, 2015

I sorted out a fix that should definitely preserve the existing behavior independent of how crazy browsers are. Hopefully it looks okay!

I have a few other things I'm working on in core, so I'll try to do a patch or minor version in the next week or so.

@KarelGGIT

This comment has been minimized.

Show comment
Hide comment
@KarelGGIT

KarelGGIT Dec 18, 2015

Apparently this problem is not solved ? I have been experimenting with Elm lately, and want to figure out how to add keyboard interactions to the current working example with a textarea and buttons.
That works, but i couldn't use Signal.merge well. I have placed a Q on StackOverflow (attachment), and got an answer.

According to the person whom answered it, his solution doesn't work on firefox (just c/p his first code block). However it does work in Chrome/Edge. The weird effect on firefox is that i'm unable to enter text in my textarea. Probably the signal goes AWOL...

I'm going to try out bjz alternative.

Attached: link to StackOverflow Q: http://stackoverflow.com/questions/34320999/combine-multiple-signals-for-one-model-using-html-events-and-keyboard-signals

KarelGGIT commented Dec 18, 2015

Apparently this problem is not solved ? I have been experimenting with Elm lately, and want to figure out how to add keyboard interactions to the current working example with a textarea and buttons.
That works, but i couldn't use Signal.merge well. I have placed a Q on StackOverflow (attachment), and got an answer.

According to the person whom answered it, his solution doesn't work on firefox (just c/p his first code block). However it does work in Chrome/Edge. The weird effect on firefox is that i'm unable to enter text in my textarea. Probably the signal goes AWOL...

I'm going to try out bjz alternative.

Attached: link to StackOverflow Q: http://stackoverflow.com/questions/34320999/combine-multiple-signals-for-one-model-using-html-events-and-keyboard-signals

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Dec 18, 2015

Contributor

It's indeed not yet fixed. See the pull request https://github.com/elm-lang/core/issues/463 linked to above, not merged.

Contributor

jvoigtlaender commented Dec 18, 2015

It's indeed not yet fixed. See the pull request https://github.com/elm-lang/core/issues/463 linked to above, not merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment