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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion core/src/display_object/edit_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2000,7 +2000,13 @@ impl<'gc> EditText<'gc> {
};

if let Avm2Value::Object(target) = self.object2() {
let character_string = AvmString::new_utf8(context.gc(), character.to_string());
let character_string = AvmString::new_utf8(
context.gc(),
match character {
Self::INPUT_NEWLINE => "\n".into(),
chr => chr.to_string(),
},
);

let mut activation = Avm2Activation::from_nothing(context);
let text_evt = Avm2EventObject::text_event(
Expand Down
40 changes: 40 additions & 0 deletions tests/tests/swfs/avm2/edittext_newline_character/Test.as
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package {
import flash.display.Sprite;
import flash.events.Event;
import flash.events.KeyboardEvent;
import flash.events.TextEvent;
import flash.text.TextField;
import flash.text.TextFieldType;

public class Test extends Sprite {
public function Test() {
var textField: TextField = new TextField();
textField.type = TextFieldType.INPUT;
textField.border = true;
textField.multiline = true;
textField.height = 200;
textField.addEventListener(Event.CHANGE, onChange);
textField.addEventListener(KeyboardEvent.KEY_DOWN, onKeyDown);
textField.addEventListener(TextEvent.TEXT_INPUT, onTextInput);
addChild(textField);
}

public function onChange(evt: Event): void {
trace("onChange");

var text: String = evt.target.text;
for (var i: int = 0; i < text.length; i++) {
trace(" U+" + text.charCodeAt(i).toString(16));
}
}

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

// trace("onKeyDown: U+" + evt.charCode.toString(16));
}

public function onTextInput(evt: TextEvent): void {
trace("onTextInput: U+" + evt.text.charCodeAt(0).toString(16));
}
}
}
7 changes: 7 additions & 0 deletions tests/tests/swfs/avm2/edittext_newline_character/input.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{ "type": "MouseDown", "pos": [10, 10], "btn": "Left" },
{ "type": "TextInput", "codepoint": "a" },
{ "type": "TextControl", "code": "Enter" },
{ "type": "TextInput", "codepoint": "b" },
{ "type": "TextControl", "code": "Enter" }
]
18 changes: 18 additions & 0 deletions tests/tests/swfs/avm2/edittext_newline_character/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
onTextInput: U+61
onChange
U+61
onTextInput: U+a
onChange
U+61
U+d
onTextInput: U+62
onChange
U+61
U+d
U+62
onTextInput: U+a
onChange
U+61
U+d
U+62
U+d
Binary file not shown.
1 change: 1 addition & 0 deletions tests/tests/swfs/avm2/edittext_newline_character/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
num_ticks = 1
Loading