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

[Web] Add IME input support #79362

Merged
merged 1 commit into from
Dec 16, 2023
Merged

[Web] Add IME input support #79362

merged 1 commit into from
Dec 16, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jul 12, 2023

@bruvzg bruvzg added this to the 4.x milestone Jul 12, 2023
@bruvzg bruvzg force-pushed the web_ime branch 3 times, most recently from 6b3e923 to f2a2101 Compare July 12, 2023 13:28
@bruvzg bruvzg force-pushed the web_ime branch 3 times, most recently from ef4d8b3 to ced2224 Compare July 26, 2023 07:14
@bruvzg bruvzg marked this pull request as ready for review July 26, 2023 07:14
@YuriSizov YuriSizov requested a review from Faless July 26, 2023 14:31
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

This looks really nice 🥳

I wonder if this can replace the experimental virtual keyboard support...

I remember I tested the composition events when doing that work, and the browser support was all over the place (missing events, repeated ones, etc) depending on both platform and vendor.

Which OS/browsers did you test?

platform/web/js/libs/library_godot_input.js Outdated Show resolved Hide resolved
@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Jul 26, 2023
@akien-mga
Copy link
Member

Would be nice to finalize this for 4.2 :)

@jaekong
Copy link

jaekong commented Dec 1, 2023

I have tested current build of your fork, and these are my findings:

  1. Currently does not work on Safari and Chromium-based browsers (I have tested it on Arc) It is essentially the same behavior with 4.2.stable, but here's a bit of a detailed explanation video.
2023-12-02.3.33.38-H.264.MP4.mp4
  1. On Firefox, it is better than stable (which does not use IME at all so it just comes out as English alphabet), but it has some weird behavior.
2023-12-02.3.39.03.mov
  • Vietnamese and French characters are duplicated.
  • When inputting Korean characters, cursor does not advance as it should - it sometimes does advance, but it seems to be random.

I am running it on macOS 14.0 Sonoma in case that is relevant.

@bruvzg
Copy link
Member Author

bruvzg commented Dec 1, 2023

Which OS/browsers did you test?

Retested it, and it seems to be working only in Firefox. Until Chromium have IME API implemented, we probably won't be able to do anything with it.

We should probably keep the reference to this instead of assigning it an ID and using selector to retrieve it (which must be page-unique, so ime is also likely to collide with user custom HTML).

Done.

Vietnamese and French characters are duplicated.
When inputting Korean characters, cursor does not advance as it should - it sometimes does advance, but it seems to be random.

Both should be fixed in the last commit (by applying the same logic as #85458 use).

@jaekong
Copy link

jaekong commented Dec 1, 2023

Retested it, and it seems to be working only in Firefox. Until Chromium have IME API implemented, we probably won't be able to do anything with it.

Out of curiosity, what APIs are missing from Chromium and Safari? compositionstart/update/end are all available in both engines and it seems like they are the only DOM events used by your PR? I am not familiar enough with Godot's source to dig into C++ code changes you've made (I tried 😭) so I might have missed a lot of things.

+ Edit: I have tested compositionstart/update/end events on the three browsers with this MDN example. At least with Korean, it seems like all three browsers are outputting the same event logs.

@jaekong
Copy link

jaekong commented Dec 1, 2023

I think I am onto something, but I am not sure.

Apparently, Safari just doesn't render any DOM element inside <canvas> so it was acting as if nothing was different.

2023-12-02.5.17.54.mov

After I just moved the element out of the canvas, it started to behave a bit better. It still is acting weird, but it might be because I am using the older build with the issues I talked above. Same behavior on Arc (Chromium) too.

It does not recognize backspace when I did that, so that's new.

@bruvzg
Copy link
Member Author

bruvzg commented Dec 1, 2023

I think I am onto something, but I am not sure.

Apparently, Safari just doesn't render any DOM element inside so it was acting as if nothing was different.

Interesting find, with the key_up / key_down event handlers added to the ime div it seems to be almost fully working in Chromium (the only issue I see is missing caret, which probably rely on canvas being focused).

@bruvzg bruvzg force-pushed the web_ime branch 2 times, most recently from 415cc74 to 7d7bd6e Compare December 1, 2023 21:07
@jaekong
Copy link

jaekong commented Dec 1, 2023

Interesting find, with the key_up / key_down event handlers added to the ime div it seems to be almost fully working in Chromium (the only issue I see is missing caret, which probably rely on canvas being focused).

How are you so fast? 😂 I was figuring out keyup keydown part and it just got fixed

It seems to work perfect so far, except I think it will be better if display mode is fixed instead of absolute. Absolute makes viewport overflow.

const ime = document.createElement("div");
      ime.className = "ime";
      ime.style.background = "none";
      ime.style.opacity = 0.0;
      ime.style.position = "fixed";

I'd submit PR if it were not just a single word change 😂 but I think it'd be more of a hassle for you to merge PR than just replacing a word.

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

This is looking great 🚀 .
We are missing some cleanup for the added node, this patch adds that.

diff --git a/platform/web/js/libs/library_godot_input.js b/platform/web/js/libs/library_godot_input.js
index ec73f3bdb3..7c070f029d 100644
--- a/platform/web/js/libs/library_godot_input.js
+++ b/platform/web/js/libs/library_godot_input.js
@@ -34,6 +34,7 @@
 
 const GodotIME = {
        $GodotIME__deps: ['$GodotRuntime', '$GodotEventListeners'],
+       $GodotIME__postset: 'GodotOS.atexit(function(resolve, reject) { GodotIME.clear(); resolve(); });',
        $GodotIME: {
                ime: null,
                active: false,
@@ -121,6 +122,13 @@ const GodotIME = {
                        GodotConfig.canvas.parentElement.appendChild(ime);
                        GodotIME.ime = ime;
                },
+
+               clear: function () {
+                       if (GodotIME.ime) {
+                               GodotIME.ime.remove();
+                               GodotIME.ime = null;
+                       }
+               },
        },
 };
 mergeInto(LibraryManager.library, GodotIME);

I've also tested this on Android (would be great if someone could test on iOS).

Chrome raise the virtual keyboard, but input doesn't work. This seems to be a problem with chrome on android not emitting composition events with some(?) virtual keyboards.

With Firefox on Android on the other end, some composition events are emitted, and input is received thought it seems to produce unreliable results when auto-completing and auto-correcting.

I don't want virtual keyboards to block this PR since that's not officially supported anyway.
I think this PR still needs some work to at least not break the optional experimental vk support we introduced some time ago which uses a similar approach.
I think we might want to disable IME completely when that option is enabled.
I'll see if I can come up with another patch.

platform/web/js/libs/library_godot_input.js Outdated Show resolved Hide resolved
platform/web/js/libs/library_godot_input.js Outdated Show resolved Hide resolved
@bruvzg bruvzg requested a review from a team as a code owner December 5, 2023 05:52
@Faless
Copy link
Collaborator

Faless commented Dec 5, 2023

Here's the other patch to only enable IME when experimental VK support is disabled (default) or not available (enabled but does not detect a touchscreen).

diff --git a/platform/web/display_server_web.cpp b/platform/web/display_server_web.cpp
index e9f5577fbe..1b63f28b08 100644
--- a/platform/web/display_server_web.cpp
+++ b/platform/web/display_server_web.cpp
@@ -1137,7 +1137,6 @@ bool DisplayServerWeb::has_feature(Feature p_feature) const {
 	switch (p_feature) {
 		//case FEATURE_GLOBAL_MENU:
 		//case FEATURE_HIDPI:
-		case FEATURE_IME:
 		case FEATURE_ICON:
 		case FEATURE_CLIPBOARD:
 		case FEATURE_CURSOR_SHAPE:
@@ -1151,6 +1150,9 @@ bool DisplayServerWeb::has_feature(Feature p_feature) const {
 		//case FEATURE_WINDOW_TRANSPARENCY:
 		//case FEATURE_KEEP_SCREEN_ON:
 		//case FEATURE_ORIENTATION:
+		case FEATURE_IME:
+			// IME does not work with experimental VK support.
+			return godot_js_display_vk_available() == 0;
 		case FEATURE_VIRTUAL_KEYBOARD:
 			return godot_js_display_vk_available() != 0;
 		case FEATURE_TEXT_TO_SPEECH:

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Amazing work 🚀 !
Thank you very much 🏆 !

@YuriSizov YuriSizov changed the title [Web] Add IME input support. [Web] Add IME input support Dec 15, 2023
@YuriSizov YuriSizov merged commit 86a4115 into godotengine:master Dec 16, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@zerosum-0
Copy link

zerosum-0 commented Jan 18, 2024

Hi
Can you check this bugs.
I made one LineEdit and one Text box .
And When I change the focus while typing.

The last letter dissapears in mac.
The last letter move to another control in web (I used chrome)

KoreanIMEBugReproduction 2.zip

2024-01-19.8.37.56.mov
2024-01-19.8.41.04.mov

@Zireael07
Copy link
Contributor

@zerosum-0 please make a separate issue report

@JunShiozawa
Copy link

I am Japanese.
Prease add milestone Godot 3.x
Godot 4.x's web export don't run on iOS and Mac Safari.

@MASoftware-es
Copy link

Good morning.
I am currently using the official stable version v4.2.1, and this bug still occurs. When I use a "TextEdit" or an "InputLine" in the application, I can introduce accents if I compile natively in desktop application mode, both for Windows, Linux, Mac, but however, when I generate an HTML project, it only works from Windows, since neither from OSX nor from Linux can I get those fields to accept accents.
It seems that the case was closed, but it continues to happen.
Can you reconsider continuing the investigation and look for an alternative?
Thank you so much.

@akien-mga
Copy link
Member

@MASoftware-es That issue was fixed in 4.3, you should test 4.3-dev5 or later.

@MASoftware-es
Copy link

MASoftware-es commented Apr 18, 2024 via email

@JunShiozawa
Copy link

@akien-mga
Why do not you add this Godot 3.x?
I need this on Godot 3.x

@akien-mga
Copy link
Member

@akien-mga Why do not you add this Godot 3.x? I need this on Godot 3.x

It could be backported to 3.x if a contributor is interested. The 3.x branch is supported on a best-effort basis, and we already have our hands full with working on improvements for 4.3 (like this PR for example), and bug fixes for 4.2. So I can't make any promises because this is a resourcing question, not a technical issue.

@JunShiozawa
Copy link

@bruvzg

I am Japanese.

Could you please implement [Web] Add IME input support in Godot 3.x ?

Web export of Godot 4.x cannot be used for Safari for iPhone.

This is a political issue for Japanese people.

@JunShiozawa
Copy link

JunShiozawa commented Jun 25, 2024

This is
Chrome for Android Japanese input OK.
Firefox for Android Japanese input OK.
Safari for iPhone Japanese input OK.
Firefox for Linux Japanese input OK.
Firefox for Windows Japanese input OK.

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

Successfully merging this pull request may close these issues.

Cannot type dead keys (accented characters) or use Input Method Editor on a LineEdit exported to HTML
10 participants