Skip to content

fix(ime): implement zcr_extended_text_input_v1 handlers and fix serial/activation bugs - #30

Draft
kkimdev wants to merge 4 commits into
google:virtwlfrom
kkimdev:wayland-ime-core-2
Draft

fix(ime): implement zcr_extended_text_input_v1 handlers and fix serial/activation bugs#30
kkimdev wants to merge 4 commits into
google:virtwlfrom
kkimdev:wayland-ime-core-2

Conversation

@kkimdev

@kkimdev kkimdev commented Jul 8, 2026

Copy link
Copy Markdown

This is PR 2 of a stacked series (split from #27 for reviewability).


Main fix: last syllable lost when committing Korean text

Steps:

  1. Switch to Korean IME input method
  2. Type "가나다" (three syllables, Enter after each: ㄱ→가→가낙→가나→가나닥→가나다)
  3. Press Enter to commit

Observed: "가나" appears in the application. "다" is lost.

Intended: "가나다" should appear in the application.

Root cause: When the ChromeOS IME sends on_commit_string followed by on_preedit_string(""), the final preedit clear was sent as a v3 preedit_string("") but with no following done event. The guest needs done after 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 done event is now sent after every preedit_string, commit_string, delete_surrounding_text, and set_preedit_region — not just after commit. A with_state helper provides a monotonically increasing done_serial for each such done event, replacing the hardcoded done(0).

confirm_preedit (Enter/Space to commit): Was a no-op. Now forwarded as v3 preedit_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 done serial fix in isolation, for example, breaks on_keysym forwarding (causes replayed stale preedit text), and confirm_preedit needs 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 v3 delete_surrounding_text + preedit_string + done, using cached surrounding_text from v3. Validates UTF-8 char boundaries. Was a no-op; ChromeOS IME calls this on every keystroke.

  • update_host_activation (new): v3 enable/disable alone doesn't tell the host when to show the IME — the host also needs wl_keyboard.enter/leave. This state machine gates activation on (enabled && active_surface != None), called from keyboard.rs enter/leave and from v3 on_commit. Without it, applying the done serial fix causes the IME to activate on the wrong surface or not at all.

  • store_host_serial + real serial in commit_state: v3 commit_state was sending serial 0, which the host skips. The serial from every v1 event is now stored and forwarded in commit_state. Required because the serial-aware done fix reveals that the host was also ignoring commit_state.

  • surrounding_text_dirty flag: on_commit was consuming surrounding_text via .take() every time, so after forwarding once, subsequent calls sent nothing. Now it only forwards when the guest actually calls on_set_surrounding_text. Without this, set_preedit_region has no text to reference after the first edit.

  • on_keysym forwarding: Forwards IME key events as wl_keyboard::key to the guest with the real host serial and time. Without this, the core done serial fix enters a stale-preedit-text regression because the host serial namespace doesn't align with the guest's expectations for key event ordering.

…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).
@InternetOfTofu

Copy link
Copy Markdown
Collaborator

Is there a sequence I should merge the 31-34 PR?

@kkimdev

kkimdev commented Jul 10, 2026

Copy link
Copy Markdown
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.

cargo build --release
(trap 'kill 0' INT; \
  cargo run --release -- wayland-test-1 & \
  sleep 5; \
  WAYLAND_DISPLAY=wayland-test-1 code)

I will convert this PR to a draft.

@kkimdev
kkimdev marked this pull request as draft July 10, 2026 23:30
@kkimdev

kkimdev commented Jul 10, 2026

Copy link
Copy Markdown
Author

I can update PRs and resolve conflicts(if any) as you merge.

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.

2 participants