fix(ime): implement zcr_extended_text_input_v1 handlers and fix serial/activation bugs - #30
Draft
kkimdev wants to merge 4 commits into
Draft
fix(ime): implement zcr_extended_text_input_v1 handlers and fix serial/activation bugs#30kkimdev wants to merge 4 commits into
kkimdev wants to merge 4 commits into
Conversation
…s, and activation state machine
Core IME correctness changes:
- Implement set_preedit_region: translates cursor-relative range into v3
delete_surrounding_text + preedit_string + done using cached surrounding_text.
- Implement confirm_preedit: commits cached preedit (non-empty) to guest via
preedit_string("") + commit_string + done; empty preedit just sends done.
- Add done_serial counter: monotonically increasing serial in every v3 done,
replacing the hardcoded 0 that broke guests validating serials.
- Propagate host_serial: store serial from v1 IME events (preedit_string,
commit_string, keysym, language, text_direction) and forward it in commit_state.
- Replace enabled_changed toggle with update_host_activation state machine:
activation is driven by (enabled && active_surface != None), called from
keyboard on_enter/on_leave and v3 on_commit.
- Implement delete_surrounding_text: convert v1 (index, length) to v3
(before_length, after_length) with correct bounds handling.
- Track surrounding_text_dirty to avoid consuming surrounding_text via .take()
on every commit — send only when the guest actually updates it.
- Forward real serial and time in on_keysym synthetic key events (was 0).
- Add cursor_end = text.len() in preedit_string (was 0).
kkimdev
marked this pull request as ready for review
July 9, 2026 00:08
InternetOfTofu
self-requested a review
July 10, 2026 01:40
This was referenced Jul 10, 2026
Collaborator
|
Is there a sequence I should merge the 31-34 PR? |
Author
|
@InternetOfTofu PR 31-34 doesn't have inter-dependencies and can be merged in any order. There will be a PR(the main bug fix of this PR) that depends on prior PRs, but I will upload it after all the dependency PRs are merged. I tested with the following command to see if there is an obvious regression for the individual PRs. I will convert this PR to a draft. |
kkimdev
marked this pull request as draft
July 10, 2026 23:30
Author
|
I can update PRs and resolve conflicts(if any) as you merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is PR 2 of a stacked series (split from #27 for reviewability).
Main fix: last syllable lost when committing Korean text
Steps:
Observed: "가나" appears in the application. "다" is lost.
Intended: "가나다" should appear in the application.
Root cause: When the ChromeOS IME sends
on_commit_stringfollowed byon_preedit_string(""), the final preedit clear was sent as a v3preedit_string("")but with no followingdoneevent. The guest needsdoneafter every state update to apply it; without it, the preedit clear is never processed, so the final commit appears alongside a stale preedit text which the guest may discard.Fix: A
doneevent is now sent after everypreedit_string,commit_string,delete_surrounding_text, andset_preedit_region— not just after commit. Awith_statehelper provides a monotonically increasingdone_serialfor each suchdoneevent, replacing the hardcodeddone(0).confirm_preedit(Enter/Space to commit): Was a no-op. Now forwarded as v3preedit_string("")+commit_string+done, so pressing Enter or Space to confirm preedit works.Other changes necessary to make the core fix work without regression:
The fixes below are interdependent with the core fix — applying the
doneserial fix in isolation, for example, breakson_keysymforwarding (causes replayed stale preedit text), andconfirm_preeditneeds activation + serial state to work correctly. They are included together to form a coherent, regression-free patch.on_set_preedit_region: Translates the extension event into v3delete_surrounding_text+preedit_string+done, using cachedsurrounding_textfrom v3. Validates UTF-8 char boundaries. Was a no-op; ChromeOS IME calls this on every keystroke.update_host_activation(new): v3enable/disablealone doesn't tell the host when to show the IME — the host also needswl_keyboard.enter/leave. This state machine gates activation on(enabled && active_surface != None), called fromkeyboard.rsenter/leave and from v3on_commit. Without it, applying thedoneserial fix causes the IME to activate on the wrong surface or not at all.store_host_serial+ real serial incommit_state: v3commit_statewas sending serial0, which the host skips. The serial from every v1 event is now stored and forwarded incommit_state. Required because the serial-awaredonefix reveals that the host was also ignoringcommit_state.surrounding_text_dirtyflag:on_commitwas consumingsurrounding_textvia.take()every time, so after forwarding once, subsequent calls sent nothing. Now it only forwards when the guest actually callson_set_surrounding_text. Without this,set_preedit_regionhas no text to reference after the first edit.on_keysymforwarding: Forwards IME key events aswl_keyboard::keyto the guest with the real host serial and time. Without this, the coredoneserial fix enters a stale-preedit-text regression because the host serial namespace doesn't align with the guest's expectations for key event ordering.