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

key may be incorrect inside an event handler #7569

Open
1 of 17 tasks
Skizy opened this issue Feb 20, 2025 · 8 comments
Open
1 of 17 tasks

key may be incorrect inside an event handler #7569

Skizy opened this issue Feb 20, 2025 · 8 comments

Comments

@Skizy
Copy link

Skizy commented Feb 20, 2025

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

1.11.3

Web browser and version

Firefox 135.0 / Chromium 133

Operating system

Linux 6.13.2

Steps to reproduce this

Steps:

  1. Press key d
  2. Press and then release key a
  3. Release key d. In keyReleased key will be equal to a, though ev.key will be correct (d).

This happens since 1.11.3. In 1.11.2 and before everything works correctly.

There's also an issue with keyPressed in Firefox (not in Chromium). It fires continuously while a key is pressed, not just once. It is probably linked to this issue, since it started happening in 1.11.3, I don't know if I should create a new issue about that too.

Snippet:

function keyPressed(ev) {
	console.log(`pressed ${ev.key} (p5.key = ${key})`);
}

function keyReleased(ev) {
	console.log(`released ${ev.key} (p5.key = ${key})`);
}
@Skizy Skizy added the Bug label Feb 20, 2025
Copy link

welcome bot commented Feb 20, 2025

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@davepagurek
Copy link
Contributor

Do you see the same repetition behaviour in this sketch? I tried it on Firefox on Mac and couldn't reproduce it, I wonder if the OS has something to do with it: https://editor.p5js.org/davepagurek/sketches/H8yqpnBWw

Also in that sketch, in index.html, I have a commented-out script tag for the 2.0 beta. If you swap out the script tags, does the repetition issue happen for you there?

For the key issue, I think we're aiming to transition to event.key and event.code rather than the global key, as trying to squish everything into a single value has always been a bit lossy.

@Skizy
Copy link
Author

Skizy commented Feb 20, 2025

I have tried it and the repetition happens for me in both 1.11.3 and 2.0 beta.

@limzykenneth
Copy link
Member

I'll try to replicate this on Linux, will report back.

@limzykenneth
Copy link
Member

limzykenneth commented Feb 22, 2025

I'm able to replicate the problem on Firefox on Linux (Firefox on Mac works as expected, on Mac is has incorrect value but not the repeated event handler issue), not sure why the event handler is being fired repeatedly though, I'll try to look into it. We'll need to possibly fix for both 1.x and 2.0.

@limzykenneth
Copy link
Member

The incorrect key value issue is due to the removal of this line as part of #7435 fixing #7282. The PR likely affected the repeat as well which I'll look into next.

@davepagurek Is that removal intentional? It is a bit up for interpretation from the reference of key what the value should be in the key down event handler since it uses the hacked together typed semantic so I'm not sure if we revert the removed line, will it change behavior of some fix that was intended in the PR?

@limzykenneth
Copy link
Member

For the repeated firing, I believe e.repeat is bugged for Firefox on linux and there are also another unsolved bug in Chrome (that I've verified) as well. We need an alternative way of detecting repeat without e.repeat.

@davepagurek
Copy link
Contributor

@davepagurek Is that removal intentional? It is a bit up for interpretation from the reference of key what the value should be in the key down event handler since it uses the hacked together typed semantic so I'm not sure if we revert the removed line, will it change behavior of some fix that was intended in the PR?

I think putting it back doesn't change the behaviour of the rest of that PR. I think it may have been due to a misunderstanding of what key should be? I think I assumed that, ideally, when one key is released, if another key is held, key should become that held key, as opposed to being sort of a shortcut for "the key the last event is about." But since there are other preferred ways to tell what keys are down, and other preferred ways to check what key the event is for, I think it's fine if we want to revert the removal of that line and get the old behaviour back.

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

No branches or pull requests

3 participants