-
-
Notifications
You must be signed in to change notification settings - Fork 881
web: Implement basic IME #19896
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
base: master
Are you sure you want to change the base?
web: Implement basic IME #19896
Conversation
This isn't working on Android when I try to type in an EditText with the virtual keyboard. |
A virtual keyboard like the one on Android |
To explain the current logic, which I guess may need comments.
|
I'm guessing the IME logic can be used to support this more properly, but landing this PR without IME support would regress the virtual keyboard. |
Maybe we can do exactly this but also keydown and keyup the |
a00d15d
to
2af747a
Compare
@danielhjacobs can you check if the current code works properly? It does for me |
Tried with GBoard and it worked perfectly. With Samsung Keyboard it's unfortunately a different story, see recording. Screen_Recording_20250329_171917_Chrome.online-video-cutter.com.mp4 |
It seems that this keyboard uses IME for inputting all text. Without implementing IME on web we cannot have both IME preview and IME input working :/ This PR breaks IME preview, but fixes IME input. |
Okay, as IME on web is a mess, I've decided to implement basic IME mechanics (including preediting) on web. @danielhjacobs @n0samu You can test it out here: https://kjarosh.github.io/ruffle/pr19896/ |
This patch implements IME preediting and committing on web. It does not implement moving the cursor and proper positioning yet.
As stated on Discord, but putting here for future people, this is now working with Samsung Keyboard. |
@jmousy Could you check if it works properly? It's available at https://kjarosh.github.io/ruffle/pr19896/ |
This is a summary of the chat on Discord.
|
if (!event || event.isComposing || event.inputType === 'insertCompositionText') { | ||
// Ignore composing events, we'll get the composed text at the end | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately for Windows IME to work we may need this:
const now = Date.now();
// If input occurs immediately (within 5ms) after compositionend, ignore it (Windows IME case)
// 5 is arbitrary, can be adjusted
if (now - lastCompositionEndTime < 5) {
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather go with implementing deleteContentBackward
than adding such hacks, but that's out of scope of this PR, as it's not IME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, it could be this I imagine, right?
if (event.inputType === "deleteContentBackward") {
for (const eventType of ["keydown", "keyup"]) {
this.element.dispatchEvent(
new KeyboardEvent(eventType, {
key: "Backspace",
bubbles: true,
}),
);
}
}
And deleteContentForward
would be the same but with the Delete
key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's that easy, as it might be more than one character to delete. We would have to make sure that it's triggered for one character only, by e.g. calling preventDefault
in beforeinput
for deleteContentBackward
when there's more than one character.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it's not the exact same as a backspace? The description certainly sounds that way:
delete the content directly before the caret position and this intention is not covered by another inputType or delete the selection with the selection collapsing to its start after the deletion
https://w3c.github.io/input-events/#interface-InputEvent-Attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Namely, "directly before the caret"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should note, even when I type the whole 가나다 on Windows, the insertText
only contains 다 and it is fired after a single deleteContentBackward
which occurs after compositionend
, where compositionend
has 가나다 as data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why "If you click on the screen after entering text "가나다", an extra "다" is entered: "가나다다""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Namely, "directly before the caret"
You can have more than one character before the caret. And note we're talking about automated events, not events caused directly by user input. If you wanted to remove the whole text inputted by IME and type it again, which event would you use for that? You're not deleting a word, not a line, there's no other inputType
to choose in this case.
You can also check Japanese IME, as it works a bit differently than Korean. I suspect Windows may delete more than one character there.
@@ -1265,7 +1278,24 @@ export class InnerPlayer { | |||
); | |||
} | |||
} | |||
input.value = ""; | |||
this.virtualKeyboard.value = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relates to advanced text input so probably needs to be handled in a follow-up, but I have discovered that by clearing the input value on every input, when you click a suggestion with Android GBoard, it does not fire the expected deleteContentBackward event(s) if the suggestion has different characters from the entered text, probably because it does not think it has to do so as there is no text in the text field.
Maybe we can store and clear the thing we want to type as a data-*
attribute instead, and leave the input alone unless a new text area gets focused, or set its value to the actual text field content from the Rust side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've been thinking about this, but that's a lot of work: you need to sync not only the text but also the caret from both sides (Rust, web). Also there's the issue of multiline fields, text restrictions, position of the text field, available glyphs, etc.
Below is the output from the link below, as requested by Daniel Jacobs on Discord: Input text: '가나다' (rkskek) iOS 18.4 (default keyboard)
Android 14 (samsung keyboard)
Android (GBoard)
Windows 11
Ubuntu 24.04
macOS 15.4
|
Explanation of issues: iOS default keyboardBecause we clear the input for each character typed, the characters do not combine, as the only input event that fires is an insertText for each individual character (all six) and there are no composition events or advanced text events. We'd need to properly handle the advanced text events anyway.iOS GBoardUnknown issue as I don't know what events it fires. Probably relates at least partially to clearing the input for each character typed.Android Samsung KeyboardNo issue, every input event occurs during composition and the data at compositionend is correct, containing all the entered text.Android GBoardFor the initially mentioned input, all input events happen during composition and the data at compositionend is correct. I'd need to see the events that fire when entering 1 + 2 + 3 + ㄱ + ㅏ + ㄴ + ㅏ + ㄷ + ㅏ to know the issue there.Windows default keyboardBecause we clear the input for each character typed, the deleteContentBackward event that is supposed to fire before a final input event duplicates the last combined character does not fire. Even if it did, we don't handle deleteContentBackward.macOS default keyboardUnsure, based on the listed events I would expect everything to work correctly. If you ignore the order, macOS seems to be typing everything twice.Linux default keyboardNo issue, there is a non-composing insertText input event containing 가 that occurs following a compositionend event with no data, a non-composing insertText input event containing 나 that occurs following a compositionend event with no data, and a compositionend event at the end containing 다 as data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JS changes approved. Functionality approved as it's a definite improvement despite not being perfect. Rust changes seem fine, I'm not confident enough to approve them myself though.
I'm about 99.9% sure we need to stop clearing the hidden input for each character typed in the future. |
I have a theory for what Mac is doing wrong, and if I'm right I wonder if this would help: this.virtualKeyboard.addEventListener("keydown", this.ignoreComposingKeyEvents.bind(this));
this.virtualKeyboard.addEventListener("keyup", this.ignoreComposingKeyEvents.bind(this));
ignoreComposingKeyEvents(event: KeyboardEvent) {
if (event.isComposing) {
event.preventDefault();
event.stopPropagation();
return;
}
} My theory is maybe Mac attempts to fire events for composing key events, causing the keyup/keydown to write text too. |
Me too. Moreover, I'm 97% sure we should just synchronize the text field with the HTML input and issue Flash events based on changes. But that's a separate issue from IME, as IME in Flash behaves differently to normal input (and it gets underlined). Even with full text synchronization we should translate IME events to Flash. |
This patch implements IME preediting and committing on web. It does not
implement moving the cursor and proper positioning yet.