Skip to content

Improve robustness of TextAgent in eframe/web#8045

Draft
umajho wants to merge 26 commits into
emilk:mainfrom
umajho:fix-cheonjiin
Draft

Improve robustness of TextAgent in eframe/web#8045
umajho wants to merge 26 commits into
emilk:mainfrom
umajho:fix-cheonjiin

Conversation

@umajho

@umajho umajho commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

This PR primarily aims to address an issue with the Samsung Keyboard Korean Cheonjiin layout found by @rustbasic.
Since I don't have access to the same environment, I'll need to rely on their feedback for validation.

Additionally, I need to gather feedback on whether these changes introduce regressions in other setups, so this PR is not ready.

See also: https://stackblitz.com/edit/egui-text-agent-experiment?file=src%2FApp.tsx

@github-actions

github-actions Bot commented Mar 29, 2026

Copy link
Copy Markdown

Preview available at https://egui-pr-preview.github.io/pr/8045-fix-cheonjiin
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

View snapshot changes at kitdiff

@umajho

umajho commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

@rustbasic Hello.

I'd appreciate it if you could test whether this solution works with the Cheonjiin IME you are using.
Since I'm unable to reproduce the bug on my end, there is a fair chance that this first attempt may not fully resolve it, though I believe the overall approach is on the right direction.

It would also be very helpful if you could try this in other web setups to check for any regressions.
(No need to test native setups, as these changes should not affect them.)

Thanks in advance.

Comment thread crates/egui/src/data/input.rs Outdated
///
/// `before_chars` and `after_chars` are the number of characters (not
/// bytes) to delete before and after the cursor, respectively.
DeleteSurrounding {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is actually not an ad hoc variant introduced just for this PR. A variant with the same name was added to winit in 0.31 (though in this implementation, the fields are defined as *_chars rather than *_bytes).

https://docs.rs/winit/0.31.0-beta.2/winit/event/enum.Ime.html#variant.DeleteSurrounding

@KonaeAkira

Copy link
Copy Markdown
Contributor

I can confirm this PR fixes the 2 issues mentioned in #8046, at least on my device.

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho
I tested it on Android + Cheonjiin IME.
The cursor position keeps moving forward, and character deletion behaves strangely.

rustbasic-2026-03-31.mp4

@umajho

umajho commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Wow, that's bad.

If you go Settings -> System -> Languages & input -> On-screen keyboard, what does it show?
For example, here is mine:

Screenshot Screenshot 2026-03-31 at 6 05 27 PM (The Android Keyboard doesn't provide Korean.)

Could you also check https://stackblitz.com/edit/egui-text-agent-experiment and record what it looks like when you input text?
For example, this is what it looks like with GBoard on Android:

Screencast

2026-03-31 6 30 00 PM

For reference, here is how GBoard behaves on the demo:

Screencast

2026-03-31 6 35 12 PM

@rustbasic

rustbasic commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Are you asking for the keyboard settings screen first? Here it is.

삼성키보드 = SAMSUNG Keyboard

image blured.

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

rustbasic-2026-03-31-21-16-53.mp4

Comment thread crates/egui/src/widgets/text_edit/builder.rs Outdated
Comment thread crates/eframe/src/web/text_agent.rs Outdated
@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

With the changes mentioned in the comment, I get the best result when I modify ImeEvent::Commit in builder.rs as follows.

However, there is still an issue: when becomes 않ㅇ, the previous Hangul character is deleted, and the entire text shifts left by one position.

                    ImeEvent::Commit(commit_text) => {
                        if commit_text == "\n" || commit_text == "\r" {
                            None
                        } else {
                            let mut ccursor = cursor_range.primary;
                            state.ime_enabled = false;

                            if !commit_text.is_empty()
                                && cursor_range.secondary.index
                                    == state.ime_cursor_range.secondary.index
                            {
                                ccursor = clear_preedit_text(text, &cursor_range);
                                text.insert_text_at(&mut ccursor, commit_text, char_limit);
                            }

                            Some(CCursorRange::one(ccursor))
                        }
                    }
rustbasic-2026-03-31-21-48-00.mp4

@umajho

umajho commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

#8045 (comment)
Are you asking for the keyboard settings screen first? Here it is.

Yes, thanks for the information.
It turns out that it is only available on Samsung phones, which totally makes sense. So this is indeed inaccessible to me.
(There is an unrelated “Samsung Smart Keyboard” App on the Play Store, which is for physical keyboards.)

#8045 (comment)

Sorry, my instructions were not clear. You need to go to the “preview” tab.
That said, I found that stackblitz runs fast on Android Firefox (my default browser), but it is somehow slow on Android Chrome. So, I'm planning to rebuild this experimental page on platforms like jsfiddle using plain HTML+JS that don't require build steps, probably tomorrow.

#8045 (comment)
With the changes mentioned in the comment, I get the best result when I modify ImeEvent::Commit in builder.rs as follows.

Thanks for the suggested changes. I will check them tomorrow.

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

rustbasic-2026-03-31-23-32-37.mp4

@umajho

umajho commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

@rustbasic I have made some changes. does this improve things?

The stackblitz page has also been updated to reflect the changes.

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

It appears to be fixable, since the issue does not occur when "않았다" is entered consecutively without any problems.
The following are the responses when it is not entered consecutively.

  1. After entering "않", wait briefly for a commit to occur, and then enter the next character.
rustbasic-2026-04-01-18-46-25.mp4
  1. After entering "안", wait briefly for a commit to occur, and then enter the next character.
rustbasic-2026-04-01-18-49-07.mp4

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

It appears to be fixable, since it works correctly when "않았다" is entered continuously.

However, if you enter "않", wait briefly for a commit to occur, and then enter the next character, "않" gets duplicated.
For example, if you enter "않", wait, and then type "ㅇ", it becomes "않않ㅇ".

Result 1-1) "않않았다"

rustbasic-2026-04-01-19-01-15.mp4

If you enter "안", wait briefly for a commit to occur, and then enter the next character, "안" gets duplicated.
In this case, the final output varies as follows.

Result 2-1) "안않았다"
Result 2-2) "안않않았다"
Result 2-3) "안ㅎㅎ았다"

rustbasic-2026-04-01-19-11-08.mp4

@umajho

umajho commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

Is this auto-committing behavior expected?

I will try to figure out how to fix the problem tomorrow.

@umajho

umajho commented Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the delayed response.
I've made a few more changes. Does this improve the situation?

Update:
I realized that the changes I was talking about here are simply reverting my earlier “further simplification” back to the version of the code suggested by rustbasic here.

# Conflicts:
#	crates/eframe/src/web/text_agent.rs
#	crates/egui/src/widgets/text_edit/builder.rs
@umajho

umajho commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

FIXME: 5ce55ad causes Gboard 14.7.09 to require an extra initial delete keystroke before ASCII character deletion behaves normally.
(Gboard 17.0.12 does not have this issue.)

Given that the bug is not present in newer Gboard versions, I propose that we just ignore it and remove the now-nonfunctional, obscure workaround that was previously added in this PR. Would anyone be against this?
Update: I think there is a way to solve this.
Please pardon me, but I will not have time to work on this until the end of the month (April).

kay-lambdadelta pushed a commit to kay-lambdadelta/egui that referenced this pull request Apr 13, 2026
… keyboard flickering on web (emilk#8078)

* Closes N/A
* Partially replaces emilk#7983
* Related: emilk#8045
* [x] I have followed the instructions in the PR template

## Details

In emilk#7983, I modified `Memory::request_focus` to interrupt any ongoing
IME composition. This fixed a bug where clicking inside an already
focused `TextEdit` failed to cancel the active composition, resulting in
duplicated text:

emilk#8045 (comment)

To avoid introducing API changes in that PR, I ensured the IME state was
reset by forcing `PlatformOutput::ime` to `None` for at least one frame.
While this works well on desktop platforms, it causes virtual keyboard
flickering on the web:

emilk#8045 (comment)

In this PR, I delegate the responsibility for handling IME composition
interruptions to integrations, allowing each integration to decide how
to interrupt compositions in a flexible manner.

### The new field `should_interrupt_composition` on `IMEOutput`.

Instead of introducing a new `OutputCommand` variant, this PR adds a new
field `should_interrupt_composition` to `IMEOutput`.

Interrupting an active composition is only meaningful when IME remains
allowed. If IME should be disabled altogether, `PlatformOutput::ime` can
simply be set to `None`.
Given this, IMO, it is more appropriate to attach the interrupt signal
to `IMEOutput` (i.e., the type of `PlatformOutput::ime`).
germ4n pushed a commit to germ4n/egui that referenced this pull request May 29, 2026
… keyboard flickering on web (emilk#8078)

* Closes N/A
* Partially replaces emilk#7983
* Related: emilk#8045
* [x] I have followed the instructions in the PR template

## Details

In emilk#7983, I modified `Memory::request_focus` to interrupt any ongoing
IME composition. This fixed a bug where clicking inside an already
focused `TextEdit` failed to cancel the active composition, resulting in
duplicated text:

emilk#8045 (comment)

To avoid introducing API changes in that PR, I ensured the IME state was
reset by forcing `PlatformOutput::ime` to `None` for at least one frame.
While this works well on desktop platforms, it causes virtual keyboard
flickering on the web:

emilk#8045 (comment)

In this PR, I delegate the responsibility for handling IME composition
interruptions to integrations, allowing each integration to decide how
to interrupt compositions in a flexible manner.

### The new field `should_interrupt_composition` on `IMEOutput`.

Instead of introducing a new `OutputCommand` variant, this PR adds a new
field `should_interrupt_composition` to `IMEOutput`.

Interrupting an active composition is only meaningful when IME remains
allowed. If IME should be disabled altogether, `PlatformOutput::ime` can
simply be set to `None`.
Given this, IMO, it is more appropriate to attach the interrupt signal
to `IMEOutput` (i.e., the type of `PlatformOutput::ime`).
umajho added 4 commits June 10, 2026 19:12
# Conflicts:
#	crates/egui/src/data/input.rs
# Conflicts:
#	crates/eframe/src/web/text_agent.rs

I let `GLM-5.2 (max)` resolve the conflicts.
@umajho

umajho commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@rustbasic Sorry to bother you. When you have a chance, could you verify that the fix still works now that this PR has been rebased onto the current main, especially after resolving the conflicts with #8083?

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

I have briefly tested #8045 on Windows 10, WASM (Chrome), and Android (Korean Cheonjiin keyboard).
I didn't find any issues on Windows 10 and WASM (Chrome), but I encountered a few problems on Android (Korean Cheonjiin keyboard).

  1. On Android, after entering text and having a new text cursor appear, if I select another area, the text cursor disappears. If I try to type characters in this state, the application freezes completely.

  2. On Android, after entering text and having a new text cursor appear, if I select another area, the text cursor fails to display and disappears. Once the text cursor vanishes, it does not reappear even if I select various different locations; it only becomes visible again when I input characters.

  3. On Android, when selecting text (making a selection block), the text cursor is positioned at the very beginning of the selected block instead of at the end of it.

Since these three symptoms occur even when #8045 is reverted (removed), they are bugs introduced in #8083.
Symptom 1 above disappears when #8045 is applied.
Symptoms 2 and 3 above do not exhibit any issues if Legacy visuals is turned on after applying #8045.

I will continue using it for a few more days and let you know if I find any other anomalies.
My suggestion is to apply #8045 to eliminate Symptom 1 first, and then resolve the remaining issues.

@umajho

umajho commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Ouch, @lucasmerlin. I'm so embarrassed of myself.
The out of bounds case you warned about here did end up happening, affecting Android IME users. (@rustbasic's Symptom 1)

The reason this PR fixed it is that, during the merge with #8083, GLM added a defensive check to ensure selection_start and selection_end are within bounds before slicing:

15fd72a#diff-180cddab275a9f2a9ba697c0c68fbf19182359aae05c360583d2484187c79815R98 (L98~L100)

        if selection_start > text_utf16.len() || selection_end > text_utf16.len() {
            return None;
        }

@umajho My suggestion is to apply #8045 to eliminate Symptom 1 first, and then resolve the remaining issues.

This PR still is not ready. Gboard 14.7.09 was working perfectly before, but this PR introduces some issues with backspace handling for it. Since IIRC it works in #8068, I want to figure out how to preserve Gboard 14.7.09's behavior while fixing the Cheonjiin issue at the same time.

So, I think opening a separate PR is the better approach.

On why this PR hasn't seen much progress:
I was waiting for #8083 to be merged before continuing work on this PR, since both modify text_agent.rs, and resolving the conflicts is a nightmare. I am also planning to make additional changes to text_agent.rs in this PR, so continuing before #8083 merged would have only created more conflicts and headaches. Since now #8083 is in main, I will resume work on this PR, and I don't expect it to be stale for another month


Unfortunately, I'm going to the dentist for some tooth issues, and there's a good chance that I will be recovering from a procedure afterward. Because of this, I probably wont' be available for at least the next 24 hours. If someone opens the fix PR, I'd really appreciate it.

@umajho

umajho commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

I think Symptom 2 is likely a bug introduced in this PR, because at the moment it doesn't seem reproducible for #8083 due to Symptom 1. Maybe the app freezing was mistaken for the cursor not showing by you?

I can't reproduce Symptom 3 on egui.rs. Or perhaps I misunderstood your reproduction steps? Could you upload a screencast?
(Maybe the Chrome version also matters? I'm using Vanadium 149.0.)

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

Even after applying #8271 and testing, Symptom 2 still occurs. Therefore, Symptom 2 is indeed an issue introduced by #8083 (and is not a simple misunderstanding caused by the app freezing).

To reproduce Symptom 3, you first need to input some characters so that a new text cursor appears, and then select (highlight) a block of text.

@rustbasic

Copy link
Copy Markdown
Contributor

screencast

DevTools.-.rustbasic.github.io_ezchat_.2026-06-28.15-22-26.mp4

@umajho

umajho commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@umajho

Even after applying #8271 and testing, Symptom 2 still occurs. Therefore, Symptom 2 is indeed an issue introduced by #8083 (and is not a simple misunderstanding caused by the app freezing).

To reproduce Symptom 3, you first need to input some characters so that a new text cursor appears, and then select (highlight) a block of text.

Ohh, I see. You mean selecting text while the IME composition is still active.
As a Chinese IME user (and I assume it is the same for Japanese IME users), I never select text before committing the composition, so I wasn't aware of this use case if it has not been not brought up.
Thanks for the explaining!

That also explains why I didn't encounter any of the three symptoms you reported. (Including the first one: although I made it sound obvious after your initial report, I don't think I wound have discovered it on my own. If you hadn't specifically mentioned “If I try to type characters in this state”, it would never have occurred to me to click elsewhere while composing.)

@umajho

umajho commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@rustbasic Just like the winit cursor position bug on Windows, which only started manifesting after the new IME composition visuals were implemented, the root cause of Symptom 2 and Symptom 3 you found have actually always been present.

The root cause is: On android Chrome, due to some complicated interaction in eframe/web (I can't reproduce this with a bare <input> element), when the agent <input> loses focus, Chrome still considers the IME composition to be active and never emits compositionend. I also supect this leaves Chrome's internal IME state inconsistent, which may further explain Symptom 1.


The root cause can actually be demonstrated using a commit from before #8083 was merged:

If you type something, commit it, and click back into the TextEdit, you get a normal cursor:
2026-06-29 8 44 05 PM

However, if you type something, click somewhere else without committing the composition, then click back into the TextEdit, you get what appears to be a selection (in reality, it is an active composition, but before #8083 there was no way to distinguish the two). If you type a number, the number is appended after the highlighted text instead of replacing it, which kinda indicates that the highlighted region is not actually a typical selection:
2026-06-29 8 44 34 PM

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

I haven't looked into the internal details closely, so I am not familiar with the technical aspects.

Please note that symptoms 2 and 3 above do not cause any issues if you set ui.style_mut().visuals.ime_composition.legacy_visuals = true.

@umajho

umajho commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@umajho

I haven't looked into the internal details closely, so I am not familiar with the technical aspects.

Please note that symptoms 2 and 3 above do not cause any issues if you set ui.style_mut().visuals.ime_composition.legacy_visuals = true.

The second screencast in my reply above demonstrates a buggy behavior that is caused by the same underlying bug of the symptoms you mentioned. I can reproduce it both on the commit prior to #8083 and on #8271 with the legacy visuals enabled.

What is happening is:

  • with the legacy visuals, the cursor is always drawn at the end of the composition, which appears correct for composing Korean.
  • with the legacy visuals, there is no visual distinction between an active composition and a selection, as a result, even if a selection is incorrectly treated as an active composition, it is not visually apparent.

This can give the impression that everything is working correctly under the legacy visuals, when in fact the underlying state is still incorrect.

@umajho

umajho commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@rustbasic could you verify whether the new commit fixes the symptoms you found? Thanks.

@rustbasic

Copy link
Copy Markdown
Contributor

@umajho

Great news!
I've tested it on Android, and all the previous symptoms have completely disappeared. Everything is working perfectly now!

I'll keep using it and let you know if I find any new issues.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants