fix(text-injection): route XWayland xdotool fallback through clipboard paste - #680
fix(text-injection): route XWayland xdotool fallback through clipboard paste#680AmirF194 wants to merge 3 commits into
Conversation
…d paste xdotool type simulates keypresses against the active X keyboard layout, so a character the layout doesn't map comes out as the wrong glyph or garbled (VocaHQ#657). The native ydotool path already avoids this by pasting via the clipboard instead of typing (_inject_with_wayland_tool); the XWayland xdotool fallback (_inject_with_xdotool) never got the same treatment. Prefer clipboard paste on that fallback too when ydotool is installed, falling back to raw xdotool type only when no paste tool is available or the paste itself fails. Fixes VocaHQ#657
jatinkrmalik
left a comment
There was a problem hiding this comment.
@AmirF194 Thank you for raising this. Did a quick adversarial review. CI is green, which I appreciate, but I do not think this fixes #657 as reported.
The #657 AppImage/GNOME log already ran _try_recover_from_fallback() and then Text injected using xdotool into Telegram (X11 window id 27262988). AppImage does not bundle ydotool (see packaging/appimage/build.sh).
On inject_text():
ydotool type ""works → recover switches toWAYLANDand_inject_with_wayland_toolalready pastes. This block never runs.- ydotool missing →
whichis false. No change. - ydotool present, daemon down →
ydotool keyalmost certainly fails too, then wexdotool typethe same garbled glyphs.
Can we drop "Fixes #657" until the xdotool-only fallback can paste without ydotool? The tool that exists on this path is xdotool. I would rather see xclip/xsel (X11 CLIPBOARD, what XWayland/Telegram actually paste) plus:
xdotool key --clearmodifiers ctrl+vydotool + wl-copy is the native Wayland recipe, not the XWayland one.
Product truth: text has to land in the focused field. Returning True skips the xdotool fallback whenever _inject_via_clipboard_paste gets rc=0 from ydotool key. That is not evidence of a paste.
Two races vs the old type path:
_inject_with_wayland_toolwaits for modifiers to release before paste, because a held PTT/Alt turns Ctrl+V into Ctrl+Alt+V (nothing pastes, command still succeeds). This caller does not wait._copy_to_clipboardpreferswl-copy. The #657 target is XWayland. Mutter/KWin clipboard bridging is async, so Ctrl+V can paste the previous X11CLIPBOARD, or nothing. We then report success.
If you keep a ydotool paste attempt at all: wait for modifiers, write the X11 clipboard (xclip/xsel) for WAYLAND_XDOTOOL, and do not treat ydotool rc=0 as "text is in the field."
The new tests call _inject_with_xdotool after overwriting environment. They never call inject_text(), so they miss recover stealing the working-ydotool case (the actual #657 log). any(c[:2] == ["ydotool", "key"]) is also true for the ydotool key --help probe, even if Ctrl+V never ran. Can we assert the paste argv and that wl-copy/xclip received the dictated string?
Also missing:
- X11
environmentstill usesxdotool type - paste failure still falls through to
xdotool type inject_text()onWAYLAND_XDOTOOLwith no ydotool still types (the reported setup)
Gateway-green pytest is not proof that привет landed in the focused XWayland field.
Happy to re-review once that path works without ydotool.
jatinkrmalik's review on VocaHQ#680 pointed out the AppImage build does not bundle ydotool, so the prior fix's ydotool clipboard paste never ran on the actual reported setup; it fell through to the same layout-dependent xdotool type as before. It also flagged a real paste-target mismatch: an XWayland window reads the X11 CLIPBOARD selection, not the Wayland clipboard wl-copy/ydotool write to. The XWayland fallback now copies via xclip/xsel and pastes with xdotool key --clearmodifiers ctrl+v when either is installed, waiting for held modifiers to release first (the same wait already used by the native ydotool path). ydotool clipboard paste stays as a second fallback when xclip/xsel are absent; xdotool type is the last resort. New regression test drives inject_text() end-to-end with no ydotool on PATH, reproducing the reported AppImage/GNOME log rather than calling the fallback method directly, so it also covers the recovery check that could otherwise skip this path.
|
Thanks, this was a real gap, appreciate the detailed read. Pushed a fix that addresses the core problem: the XWayland fallback now copies via xclip/xsel (X11 CLIPBOARD, what Telegram/XWayland actually reads) and pastes with Also added the modifier-release wait before the paste (same helper On the test critique: added a new test that drives One thing I left alone: native X11 ( Full suite plus black/isort/flake8 green locally. Let me know if you'd like anything else changed. |
|
Pushed the fix for the changes you requested a week ago (963c65c) and don't seem to have a way to re-request your review as an outside contributor, so flagging it here instead: could you take another look when you have a chance? |
✅ Deploy Preview for voca-linux canceled.
|
|
@AmirF194 It seems like the CI is still red and failing on couple of steps. Would you mind fixing them? |
|
Thanks for merging main in. Vocalinux CI finished green on the merge commit (aff6833, completed 06:15:27Z), nothing red on my end to fix right now. Let me know if you'd like anything else addressed. |
Sorry, seems like there are merge conflicts. |
Description
On the XWayland fallback path (
DesktopEnvironment.WAYLAND_XDOTOOL),_inject_with_xdotool()always types text withxdotool type, which simulates keypresses against the active X keyboard layout. A character the current layout doesn't map (or maps differently) comes out as the wrong glyph, garbling non-Latin dictation whenever the active layout doesn't match the dictated language.The native ydotool path already avoids this:
_inject_with_wayland_tool()prefers clipboard + simulated Ctrl+V overydotool type, because a clipboard paste is layout-independent. The XWayland xdotool fallback never got the same treatment, even though it shares the same underlying problem.This PR routes
_inject_with_xdotool()through_inject_via_clipboard_paste()first when running on the XWayland fallback andydotoolis installed, falling back to rawxdotool typeonly when no paste tool is available or the paste itself fails (mirroring the existing pattern in_inject_with_wayland_tool()).Scope: this only touches the
WAYLAND_XDOTOOLbranch. A genuinely native X11 session (DesktopEnvironment.X11) is unaffected and keeps usingxdotool typeas before, since #657 is specifically about the XWayland fallback.Related Issue
Fixes #657
Type of Change
Verification
tests/test_text_injector.py:test_inject_with_xdotool_xwayland_prefers_clipboard_pastefails against unmodifiedmain(assertsxdotool typeis never called when ydotool is available) and passes on this branch;test_inject_with_xdotool_xwayland_falls_back_without_ydotoolconfirms the fallback path is unchanged when noydotoolis installed.tests/suite (pytest) passes on Python 3.9 and 3.13 (the matrix's low and high ends), run as a non-root user;flake8 --select=E9,F63,F7,F82,black --check, andisort --check-only --profile blackare all clean on both. Did not separately run 3.10/3.11; the change is plain Python with no version-specific syntax.ydotool/ydotoold(no display server or uinput device in this environment), so the fix is verified at the subprocess-call level (clipboard copy +ydotool keyinvoked,xdotool typenot invoked), not end-to-end against a live compositor.Checklist