text-input: a done the compositor sent before our disable is applied as current (the serial is ignored) #188

New issue

Open

opened 2026-10-31 23:54:52 +00:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-10-31 23:54:52 +00:00

Owner

Device::text_input_done applies every done to textInputWindow as if it were current. The protocol's serial — the number of commit requests the client has sent, which the compositor echoes in done — is never compared, and a disable (SetTextInput(false)) clears shown and the pending flags but not textInputWindow (only the leave does). So a done the compositor relayed before it processed our disable + commit — already in the socket when the event that made us disable (a tap, a key) was dispatched — reaches the application as a live edit.

How it bites a consumer (imsd-messages' pre-push review, 2026-10-31): the message field commits the composing text itself (ComposeCommitPreedit), disables the text input, and moves the cursor to the tap. The stale commit_string("wor ") + done then lands through onTextInput at the new cursor: hello wor becomes wor hello wor, the word twice and the space misplaced. It needs two thumbs (an on-screen-keyboard key and a tap on the field in one dispatch) or a click while typing; the damage is draft text, visible before sending. imsd-messages ships 1.0 with it waived (2026-11-01) and this issue as the follow-up.

Where (at 6f471ca; unchanged by 44c913f): implementations/Crafter.Graphics-Device.cpp — text_input_done (~~:423–437) invokes onTextInput / onPreedit for textInputWindow without looking at serial; the disable path (~~:455–464) clears shown and the pending flags only; the leave (~:391–401) is the one place textInputWindow is cleared.

Protocol: text-input-unstable-v3 — done's serial is the count of commit requests; a done whose serial differs from the client's count "must proceed with evaluating and applying the changes as normal" (lib/text-input-unstable-v3-client-protocol.h ~:543–546), its first step being "replace existing preedit string" — which the client can only do if it knows the done predates its own reset.

Proposed cure: count the commit requests in Device (one per enable/disable/commit we send) and remember the count at the latest disable; on done, a serial below that count marks the batch stale. Either drop stale batches in the library (simplest; a client that already committed the preedit loses only what the input method added after it, a space), or surface them flagged — a stale flag on the event, or a separate event — so a consumer can drop a stale preedit and treat a stale commit as replacing the span it committed itself. A test can drive text_input_commit_string + text_input_done with a stale serial after SetTextInput(false) on a default-constructed Window, as tests/MouseScroll drives the pointer callbacks.

`Device::text_input_done` applies every `done` to `textInputWindow` as if it were current. The protocol's serial — the number of `commit` requests the client has sent, which the compositor echoes in `done` — is never compared, and a disable (`SetTextInput(false)`) clears `shown` and the pending flags but not `textInputWindow` (only the leave does). So a `done` the compositor relayed *before* it processed our disable + commit — already in the socket when the event that made us disable (a tap, a key) was dispatched — reaches the application as a live edit. **How it bites a consumer** (imsd-messages' pre-push review, 2026-10-31): the message field commits the composing text itself (`ComposeCommitPreedit`), disables the text input, and moves the cursor to the tap. The stale `commit_string("wor ")` + `done` then lands through `onTextInput` at the new cursor: `hello wor` becomes `wor hello wor`, the word twice and the space misplaced. It needs two thumbs (an on-screen-keyboard key and a tap on the field in one dispatch) or a click while typing; the damage is draft text, visible before sending. imsd-messages ships 1.0 with it waived (2026-11-01) and this issue as the follow-up. **Where** (at 6f471ca; unchanged by 44c913f): `implementations/Crafter.Graphics-Device.cpp` — `text_input_done` (~:423–437) invokes `onTextInput` / `onPreedit` for `textInputWindow` without looking at `serial`; the disable path (~:455–464) clears `shown` and the pending flags only; the leave (~:391–401) is the one place `textInputWindow` is cleared. **Protocol**: text-input-unstable-v3 — `done`'s `serial` is the count of `commit` requests; a `done` whose serial differs from the client's count "must proceed with evaluating and applying the changes as normal" (`lib/text-input-unstable-v3-client-protocol.h` ~:543–546), its first step being "replace existing preedit string" — which the client can only do if it knows the `done` predates its own reset. **Proposed cure**: count the `commit` requests in `Device` (one per enable/disable/commit we send) and remember the count at the latest disable; on `done`, a serial below that count marks the batch stale. Either drop stale batches in the library (simplest; a client that already committed the preedit loses only what the input method added after it, a space), or surface them flagged — a `stale` flag on the event, or a separate event — so a consumer can drop a stale preedit and treat a stale commit as replacing the span it committed itself. A test can drive `text_input_commit_string` + `text_input_done` with a stale serial after `SetTextInput(false)` on a default-constructed `Window`, as `tests/MouseScroll` drives the pointer callbacks.

jorijnvdgraaf referenced this issue

2026-11-01 20:10:09 +00:00

FontAtlas: a query for whether a frame is owed, for consumers that draw on demand (a4be6f1's eviction) #190