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

avm2: Use Line Feed character for textInput newline events #19517

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

evilpie
Copy link
Collaborator

@evilpie evilpie commented Feb 14, 2025

Fixes #19482

@adrian17 adrian17 added text Issues relating to text rendering/input T-fix Type: Bug fix (in something that's supposed to work already) A-core Area: Core player, where no other category fits labels Feb 14, 2025
@adrian17 adrian17 requested a review from kjarosh February 14, 2025 20:27
}

public function onKeyDown(evt: KeyboardEvent): void {
// TODO: We must not be firing this event yet.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this comment mean? Why can't you just fire the event?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I misunderstood the comment. I thought we cannot fire the event here somehow.

The key down/up events are separate from text input events, you need to add them to input.json.

Copy link
Collaborator Author

@evilpie evilpie Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a way of producing the TEXT_INPUT and KEY_DOWN event with just one line in input.json? It seems a bit pointless to test if both results are basically hardcoded. (We also don't seem to support specifying a character for test KeyDown events, which is another problem)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a way of producing the TEXT_INPUT and KEY_DOWN event with just one line in input.json?

Nope

It seems a bit pointless to test if both results are basically hardcoded.

Text input is inherently different from key up/down, because it goes through the OS (keyboard layout, mapping, etc.). We cannot infer one from the other, it's done outside of Ruffle.

We also don't seem to support specifying a character for test KeyDown events, which is another problem

I agree, I remember adding key char as an optional field in the automated event. Maybe I'll find the commit

Copy link
Member

@kjarosh kjarosh Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I remember now. I worked on redoing key events in Ruffle to properly support key mapping. I believe we should do it first, because key code can be inferred from key char (contrary to text input or maybe, when we have key char, I'm actually not sure).

TL;DR we need to refactor input a bit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already) text Issues relating to text rendering/input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiline TextField + TextEvent.TEXT_INPUT: event.text should be \n instead of \r when user press enter
3 participants