Skip to content

protocols/text-input-v3: send done in response to every commit - #16571

Closed
hjanuschka wants to merge 1 commit into
hyprwm:mainfrom
hjanuschka:textinput-v3-ack-commits
Closed

hjanuschka wants to merge 1 commit into
hyprwm:mainfrom
hjanuschka:textinput-v3-ack-commits

Conversation

@hjanuschka

Copy link
Copy Markdown
Contributor

Describe your PR, what does it fix/add?

zwp_text_input_v3.done was only sent from updateIMEState, i.e. when the input method pushes preedit/commit back to the client. The protocol spec requires clients to withhold all pending state until they receive a done with a serial matching their commit count:

All pending state requests (set_surrounding_text, set_content_type and set_cursor_rectangle) on the zwp_text_input_v3 object should be sent and committed after receiving a zwp_text_input_v3.done event with a matching serial.

Clients implementing this strictly (e.g. Chromium, which gates on done_serial == commit_count) can therefore never update the cursor rectangle after their first commit unless the IME happens to push state back. Observable result with Chromium + fcitx5 on Hyprland: the IME candidate popup opens anchored to a stale rectangle (often the initial 0,0) and only corrects itself after a focus leave/enter (e.g. fullscreen toggle), because that is the only moment Chromium can bundle the current rectangle with enable.

This sends done(serial) in response to every commit in the protocol handler, which also covers the case where no IME is attached. The later IME-driven done from updateIMEState stays valid (same serial, carries the preedit/commit events).

Checklist

  • I have tested my changes
  • My changes are not breaking (if they are, discussion is needed)

Tested by running this build nested with WAYLAND_DEBUG=client chromium --ozone-platform=wayland:

Before (0.56.2): zero done events across the whole session, set_cursor_rectangle stuck at its first value.

After:

-> zwp_text_input_v3#52.set_surrounding_text("...", 0, 0)
-> zwp_text_input_v3#52.commit()
zwp_text_input_v3#52.done(1)
-> zwp_text_input_v3#52.set_surrounding_text("...", 0, 43)
-> zwp_text_input_v3#52.commit()
zwp_text_input_v3#52.done(2)

Related: this surfaced while debugging https://issues.chromium.org/issues/574322335 (Chromium sending the rectangle unscaled, fixed on the Chromium side); the missing acks here are an independent issue that keeps even a correct rectangle stale.

The done event was only sent from updateIMEState, i.e. when the input
method pushes preedit/commit back to the client. Per the protocol spec,
clients must withhold all pending state (set_cursor_rectangle,
set_surrounding_text, set_content_type) until they receive a done event
with a serial matching their commit count. Clients implementing this
strictly (e.g. Chromium) could therefore never update the cursor
rectangle after their first commit unless the IME happened to push
state, leaving IME candidate popups anchored to a stale rectangle.

Ack every commit with done(serial) in the protocol handler, which also
covers the case where no IME is attached at all.
@github-actions

Copy link
Copy Markdown

Hi @hjanuschka,

Thanks for your contribution! Unfortunately, Hyprland and hypr* no longer
accept contributions from unvouched contributors.

To get vouched, please see our contributing guide at wiki.hypr.land/Contributing-and-Debugging

This PR has been closed automatically and will not be re-opened.

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.

1 participant