From d5656b8060fbdb745f9ec498557c079deadf95b1 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sat, 12 Sep 2026 14:16:10 -0600 Subject: [PATCH] fix(text-injection): route XWayland xdotool fallback through clipboard paste (#680) * fix(text-injection): route XWayland xdotool fallback through clipboard 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 (#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 #657 * fix(text-injection): X11 clipboard paste for the XWayland fallback jatinkrmalik's review on #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. * fix(text-injection): thread x11_only through clipboard read/clear so restore matches the paste backend jatinkrmalik's rebase pulled in the layout-aware paste work from #788, and greptile's review on the merged branch caught a real gap in this PR's own XWayland path: _copy_to_clipboard already takes x11_only to write the X11 CLIPBOARD via xclip/xsel (since that is what an xdotool paste actually reads), but _read_clipboard and _clear_clipboard never gained the same parameter. On a Wayland host, _read_clipboard prefers wl-paste, so the "previous clipboard" saved before an XWayland paste could be read from an unrelated Wayland clipboard, and restoring it back through _copy_to_clipboard with no x11_only wrote to that same wrong backend, leaving the X11 CLIPBOARD stuck on the dictated text. Threads x11_only through both helpers and every restore call site (the immediate-failure path and the delayed _restore closure), so an XWayland paste captures, clears, and restores the same X11 CLIPBOARD selection it wrote to. * fix(text-injection): annotate clipboard helpers and XWayland paste tests Greptile P2 on #680: give `_get_clipboard_tools` a return annotation and mark the new XWayland clipboard-paste regression tests `-> None`. * fix(text-injection): derive XWayland paste from environment Drop the ydotool/wl-copy second fallback on WAYLAND_XDOTOOL (wrong clipboard, and rc=0 is not a paste) and the x11_only flag stack. Paste picks xclip/xsel + xdotool from environment; user-facing copy stays on the session clipboard. * fix(text-injection): always pass host_env() to the paste chord The AST guard only recognizes a direct host_env() call. The ternary around it looked like we were handing the AppImage library path to xdotool. * test(text-injection): give XWayland injector stubs clipboard restore state _inject_with_xdotool now pastes first, and that path reads _clipboard_restore_target. The __new__ helper in test_text_injector_ext never set it. * fix(text-injection): skip clipboard restore on a stale paste failure If a newer paste has already copied its text, the older paste's failure handler must not write its saved clipboard back. Success restores already honored the generation counter; the failure path did not. * fix(text-injection): keep original clipboard across a stale paste failure Publish the restore target as soon as copy succeeds (first writer wins) so an overlapping paste inherits it. If the older paste then fails, the newer restore still writes the pre-first clipboard, not intermediate dictated text. --------- Co-authored-by: Jatin K Malik --- src/vocalinux/text_injection/text_injector.py | 122 +++++-- tests/test_clipboard_restore.py | 132 +++++++- tests/test_text_injector.py | 314 ++++++++++++++++-- tests/test_text_injector_ext.py | 2 + 4 files changed, 497 insertions(+), 73 deletions(-) diff --git a/src/vocalinux/text_injection/text_injector.py b/src/vocalinux/text_injection/text_injector.py index 0cc0be0..62683ce 100644 --- a/src/vocalinux/text_injection/text_injector.py +++ b/src/vocalinux/text_injection/text_injector.py @@ -14,7 +14,7 @@ import subprocess import threading import time from enum import Enum -from typing import Optional # noqa: F401 +from typing import Optional from ..utils.host_process import host_env from ..utils.paths import config_dir @@ -747,7 +747,7 @@ class TextInjector: self._ibus_init_failed = True logger.warning(f"IBus initialization failed: {e}, continuing with fallback") - def _get_clipboard_tools(self): + def _get_clipboard_tools(self) -> list: tools = [] # Prefer wl-copy on Wayland (including Flatpak with --socket=wayland). host_is_wayland = ( @@ -765,6 +765,10 @@ class TextInjector: tools.append("wl-copy") return tools + def _x11_clipboard_tools(self) -> list: + """xclip/xsel only: the X11 CLIPBOARD an xdotool paste actually reads.""" + return [name for name in ("xclip", "xsel") if shutil.which(name)] + def _run_clipboard_command(self, tool: str, text: str) -> bool: # NB: wl-copy/xclip/xsel fork a background process that keeps owning the # selection in order to serve it. That child inherits our pipes, so @@ -949,7 +953,7 @@ class TextInjector: logger.debug("No better tools available, continuing with xdotool fallback") return False - def _copy_to_clipboard(self, text: str) -> bool: + def _copy_to_clipboard(self, text: str, tools: Optional[list] = None) -> bool: """ Copy text to clipboard. @@ -959,13 +963,16 @@ class TextInjector: Args: text: The text to copy to clipboard + tools: Clipboard binaries to try. Defaults to ``_get_clipboard_tools`` + (session clipboard). XWayland paste passes xclip/xsel so the + text lands on the X11 CLIPBOARD that xdotool Ctrl+V reads. Returns: True if clipboard copy was successful, False otherwise """ logger.info("Copying text to clipboard") - for tool in self._get_clipboard_tools(): + for tool in tools if tools is not None else self._get_clipboard_tools(): if self._clipboard_tool_health.get(tool) is False: continue @@ -988,7 +995,7 @@ class TextInjector: ) return False - def _clear_clipboard(self) -> bool: + def _clear_clipboard(self, tools: Optional[list] = None) -> bool: """ Clear the clipboard using the first available tool. @@ -997,9 +1004,14 @@ class TextInjector: - xsel: ``--clear`` flag - xclip: pipe empty input (creates an empty text offer) + Args: + tools: Clipboard binaries to try. Defaults to ``_get_clipboard_tools``. + Pass the same list the matching copy used (xclip/xsel for an + XWayland paste) so restore hits the same selection. + Returns True if the clipboard was cleared successfully. """ - for tool in self._get_clipboard_tools(): + for tool in tools if tools is not None else self._get_clipboard_tools(): if self._clipboard_tool_health.get(tool) is False: continue try: @@ -1212,21 +1224,29 @@ class TextInjector: """ Inject text using xdotool for X11 environments. + On WAYLAND_XDOTOOL, try an X11 clipboard paste first. ``xdotool type`` + follows the active layout and garbles non-Latin text (#657). + Args: text: The text to inject """ - # Create environment with explicit X11 settings for Wayland compatibility env = os.environ.copy() if self.environment == DesktopEnvironment.WAYLAND_XDOTOOL: - # Force X11 backend for XWayland env["GDK_BACKEND"] = "x11" env["QT_QPA_PLATFORM"] = "xcb" - # Ensure DISPLAY is set correctly for XWayland - if "DISPLAY" not in env or not env["DISPLAY"]: + if not env.get("DISPLAY"): env["DISPLAY"] = ":0" - + # xclip reads process env, not the type-path subprocess env. + os.environ.setdefault("DISPLAY", env["DISPLAY"]) logger.debug(f"Using XWayland with DISPLAY={env['DISPLAY']}") + self._wait_for_modifiers_released() + if self._inject_via_clipboard_paste(text, env=env): + return + logger.warning( + "Clipboard paste failed, falling back to xdotool type " + "(character-by-character; text may be scrambled on non-US layouts)" + ) # Add a small delay to ensure text is injected properly time.sleep(0.3) # Increased delay for better reliability @@ -1360,7 +1380,11 @@ class TextInjector: Requests text MIME types so image/file data is not treated as text. Returns the text, "" if a tool reports a verifiably empty clipboard, or None if unreadable as text (non-text data, no tool, or error). + + On WAYLAND_XDOTOOL this reads the X11 CLIPBOARD (xclip/xsel), matching + the selection the xdotool paste wrote. wl-paste is a different clipboard. """ + prefer_x11 = self.environment == DesktopEnvironment.WAYLAND_XDOTOOL host_is_wayland = ( self._session_environment == DesktopEnvironment.WAYLAND or os.environ.get("XDG_SESSION_TYPE", "").lower() == "wayland" @@ -1369,13 +1393,13 @@ class TextInjector: # Bare `xclip -o` / `wl-paste` can return raw image bytes with rc=0. candidates: list[list[str]] = [] - if host_is_wayland and shutil.which("wl-paste"): + if not prefer_x11 and host_is_wayland and shutil.which("wl-paste"): candidates.append(["wl-paste", "--no-newline", "--type", "text"]) if shutil.which("xclip"): candidates.append(["xclip", "-selection", "clipboard", "-o", "-t", "UTF8_STRING"]) if shutil.which("xsel"): candidates.append(["xsel", "--clipboard", "--output"]) - if not host_is_wayland and shutil.which("wl-paste"): + if not prefer_x11 and not host_is_wayland and shutil.which("wl-paste"): candidates.append(["wl-paste", "--no-newline", "--type", "text"]) saw_empty = False @@ -1437,7 +1461,7 @@ class TextInjector: logger.debug(f"Terminal paste detection failed: {exc}") return False - def _inject_via_clipboard_paste(self, text: str) -> bool: + def _inject_via_clipboard_paste(self, text: str, env: Optional[dict] = None) -> bool: """ Inject text by copying to clipboard and simulating a paste chord. @@ -1446,11 +1470,18 @@ class TextInjector: that chord instead. Workaround for ydotool's US-ASCII-only key events (see issue #362). The chord itself is sent with wtype keysyms when that tool is usable, otherwise with a layout-resolved ydotool keycode - for Latin 'v' (issue #787). Saves the previous clipboard and restores - it after a short delay. Overlapping pastes share one restore target + for Latin 'v' (issue #787). On WAYLAND_XDOTOOL the copy uses xclip/xsel + and the chord is xdotool's, because the focused XWayland window pastes + the X11 CLIPBOARD (#657). Saves the previous clipboard and restores it + after a short delay. Overlapping pastes share one restore target (pre-first-injection content) and a generation counter so stale restore threads exit. + Args: + text: The text to inject. + env: Subprocess environment for the paste chord. XWayland passes the + DISPLAY=:0 env the type path already builds. + Returns: True if successful, False otherwise """ @@ -1459,6 +1490,9 @@ class TextInjector: "(saving clipboard to restore after paste)" ) + prefer_x11 = self.environment == DesktopEnvironment.WAYLAND_XDOTOOL + tools = self._x11_clipboard_tools() if prefer_x11 else None + # Inherit a pending restore target so a second paste in the delay window # still restores the original clipboard, not intermediate dictated text. with self._state_lock: @@ -1467,17 +1501,26 @@ class TextInjector: pending_target if pending_target is not None else self._read_clipboard() ) - if not self._copy_to_clipboard(text): + if not self._copy_to_clipboard(text, tools=tools): logger.warning("Could not copy text to clipboard for paste injection") return False - # Clipboard is overwritten — cancel any in-flight restore and take ownership. + # Clipboard is overwritten — take ownership. Publish the original + # clipboard now (first writer wins) so an overlapping paste inherits + # it even if this paste later fails. with self._state_lock: self._clipboard_restore_generation += 1 generation = self._clipboard_restore_generation + if ( + previous_clipboard is not None + and not self._should_copy_to_clipboard() + and self._clipboard_restore_target is None + ): + self._clipboard_restore_target = previous_clipboard - # Simulate paste. Prefer wtype keysyms (layout-independent) when that - # tool is usable; otherwise ydotool. ydotool syntax differs by version: + # Simulate paste. XWayland uses xdotool against the X11 CLIPBOARD; + # native Wayland prefers wtype keysyms, then layout-resolved ydotool. + # ydotool syntax differs by version: # - 0.1.x (distro packages): named sequences, e.g. ctrl+v # - 1.x (Flatpak build): keycode:value (29=LEFTCTRL, 42=LEFTSHIFT, # and the evdev code that produces Latin 'v' on the active layout — @@ -1493,7 +1536,12 @@ class TextInjector: paste_cmd, ) subprocess.run( - paste_cmd, check=True, stderr=subprocess.PIPE, text=True, timeout=3, env=host_env() + paste_cmd, + check=True, + stderr=subprocess.PIPE, + text=True, + timeout=3, + env=host_env(env), ) logger.info(f"Text injected via clipboard paste: '{text[:20]}...' ({len(text)} chars)") except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e: @@ -1503,35 +1551,41 @@ class TextInjector: if paste_cmd: self._ydotool_release_paste_keys(paste_cmd) with self._state_lock: - if generation == self._clipboard_restore_generation: + stale = generation != self._clipboard_restore_generation + if not stale: self._clipboard_restore_target = None - if previous_clipboard is not None and not self._should_copy_to_clipboard(): + if ( + not stale + and previous_clipboard is not None + and not self._should_copy_to_clipboard() + ): if previous_clipboard == "": - self._clear_clipboard() + self._clear_clipboard(tools=tools) else: - self._copy_to_clipboard(previous_clipboard) + self._copy_to_clipboard(previous_clipboard, tools=tools) return False # Delayed restore so Ctrl+V can land first. Skip when the user wants # dictated text left on the clipboard (copy_to_clipboard setting). if previous_clipboard is not None and not self._should_copy_to_clipboard(): - with self._state_lock: - self._clipboard_restore_target = previous_clipboard def _restore() -> None: time.sleep(0.3) with self._state_lock: if generation != self._clipboard_restore_generation: return + target = self._clipboard_restore_target self._clipboard_restore_target = None + if target is None: + return # User copied something else during the delay — leave it alone. if self._read_clipboard() != text: logger.debug("Clipboard changed during restore delay; skipping restore") return - if previous_clipboard == "": - success = self._clear_clipboard() + if target == "": + success = self._clear_clipboard(tools=tools) else: - success = self._copy_to_clipboard(previous_clipboard) + success = self._copy_to_clipboard(target, tools=tools) if success: logger.debug("Clipboard restored to previous content") else: @@ -1737,9 +1791,13 @@ class TextInjector: def _clipboard_paste_command(self, *, terminal: bool = False) -> list: """Return argv for the clipboard paste chord. - Prefers wtype keysyms when that tool is usable (layout-independent). - Otherwise uses ydotool with a layout-resolved keycode for 'v'. + On WAYLAND_XDOTOOL, xdotool against the X11 CLIPBOARD (what an XWayland + window pastes from). Otherwise prefers wtype keysyms, then ydotool + with a layout-resolved keycode for 'v'. """ + if self.environment == DesktopEnvironment.WAYLAND_XDOTOOL: + chord = "ctrl+shift+v" if terminal else "ctrl+v" + return ["xdotool", "key", "--clearmodifiers", chord] if self._wtype_usable_for_paste(): return self._wtype_paste_command(terminal=terminal) return self._ydotool_ctrl_v_command(terminal=terminal) diff --git a/tests/test_clipboard_restore.py b/tests/test_clipboard_restore.py index 2e97d0f..d7a4390 100644 --- a/tests/test_clipboard_restore.py +++ b/tests/test_clipboard_restore.py @@ -56,6 +56,27 @@ class TestReadClipboard(unittest.TestCase): ["wl-paste", "--no-newline", "--type", "text"], ) + def test_skips_wl_paste_on_xwayland_fallback(self) -> None: + """WAYLAND_XDOTOOL reads the X11 CLIPBOARD, not wl-paste.""" + from vocalinux.text_injection.text_injector import DesktopEnvironment + + obj = _make_injector() + obj.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.dict(os.environ, {"WAYLAND_DISPLAY": "wayland-0"}): + with patch("vocalinux.text_injection.text_injector.shutil.which") as mock_which: + mock_which.side_effect = lambda cmd: ( + f"/usr/bin/{cmd}" if cmd in ("wl-paste", "xclip") else None + ) + with patch("vocalinux.text_injection.text_injector.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="from-x11") + result = obj._read_clipboard() + self.assertEqual(result, "from-x11") + mock_run.assert_called_once() + self.assertEqual( + mock_run.call_args.args[0], + ["xclip", "-selection", "clipboard", "-o", "-t", "UTF8_STRING"], + ) + def test_xclip_requests_utf8_string_target(self): """xclip must request UTF8_STRING so image clipboards are not read as text.""" obj = _make_injector() @@ -201,7 +222,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): obj, "_read_clipboard", side_effect=["original clipboard", "injected text"] ): with patch.object( - obj, "_copy_to_clipboard", side_effect=lambda t: copy_calls.append(t) or True + obj, "_copy_to_clipboard", side_effect=lambda t, **kw: copy_calls.append(t) or True ): with patch.object( obj, @@ -229,12 +250,12 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_read_clipboard", - side_effect=lambda: call_order.append("read") or "prev", + side_effect=lambda **kw: call_order.append("read") or "prev", ): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: call_order.append(f"copy:{t}") or True, + side_effect=lambda t, **kw: call_order.append(f"copy:{t}") or True, ): with patch.object( obj, @@ -262,7 +283,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -296,7 +317,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -325,7 +346,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -384,7 +405,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -407,7 +428,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): obj.wayland_tool = "ydotool" copy_calls: list[str] = [] - def fake_copy(t: str) -> bool: + def fake_copy(t: str, **kwargs) -> bool: copy_calls.append(t) # injection copy succeeds; restore copy fails return t != "previous_content" @@ -443,7 +464,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -471,7 +492,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -498,7 +519,9 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object(obj, "_read_clipboard", side_effect=["", "new text"]): with patch.object(obj, "_copy_to_clipboard", return_value=True): with patch.object( - obj, "_clear_clipboard", side_effect=lambda: clear_called.append(True) or True + obj, + "_clear_clipboard", + side_effect=lambda **kw: clear_called.append(True) or True, ): with patch.object( obj, @@ -529,7 +552,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -559,7 +582,7 @@ class TestClipboardRestoreAfterInjection(unittest.TestCase): with patch.object( obj, "_copy_to_clipboard", - side_effect=lambda t: copy_calls.append(t) or True, + side_effect=lambda t, **kw: copy_calls.append(t) or True, ): with patch.object( obj, @@ -665,7 +688,9 @@ class TestClearClipboard(unittest.TestCase): with patch.object(obj, "_read_clipboard", return_value=""): with patch.object(obj, "_copy_to_clipboard", return_value=True): with patch.object( - obj, "_clear_clipboard", side_effect=lambda: clear_called.append(True) or True + obj, + "_clear_clipboard", + side_effect=lambda **kw: clear_called.append(True) or True, ): with patch.object( obj, @@ -764,9 +789,9 @@ class TestOverlappingClipboardRestore(unittest.TestCase): # clipboard); pending-target must keep restoring to "URL". read_values = iter(["URL", "world"]) - with patch.object(obj, "_read_clipboard", side_effect=lambda: next(read_values)): + with patch.object(obj, "_read_clipboard", side_effect=lambda **kw: next(read_values)): with patch.object( - obj, "_copy_to_clipboard", side_effect=lambda t: copy_calls.append(t) or True + obj, "_copy_to_clipboard", side_effect=lambda t, **kw: copy_calls.append(t) or True ): with patch.object( obj, @@ -788,6 +813,81 @@ class TestOverlappingClipboardRestore(unittest.TestCase): self.assertNotIn("hello", copy_calls[2:]) self.assertIsNone(obj._clipboard_restore_target) + def test_failed_older_paste_does_not_restore_over_newer(self) -> None: + """A stale paste failure must not rewind the clipboard over a newer copy.""" + obj = _make_injector() + copy_calls: list[str] = [] + nested = {"done": False} + + def run_side_effect(cmd, **kwargs): + if not nested["done"]: + nested["done"] = True + self.assertTrue(obj._inject_via_clipboard_paste("world")) + raise subprocess.CalledProcessError(1, cmd) + return MagicMock(returncode=0) + + with patch.object(obj, "_read_clipboard", side_effect=["URL", "hello"]): + with patch.object( + obj, "_copy_to_clipboard", side_effect=lambda t, **kw: copy_calls.append(t) or True + ): + with patch.object(obj, "_should_copy_to_clipboard", return_value=False): + with patch.object(obj, "_should_use_terminal_paste", return_value=False): + with patch.object( + obj, + "_clipboard_paste_command", + return_value=["ydotool", "key", "ctrl+v"], + ): + with patch("threading.Thread"): + with patch( + "vocalinux.text_injection.text_injector.subprocess.run", + side_effect=run_side_effect, + ): + self.assertFalse(obj._inject_via_clipboard_paste("hello")) + # Stale failure must not rewind to "URL" over "world". + self.assertEqual(copy_calls, ["hello", "world"]) + self.assertNotIn("URL", copy_calls) + + def test_stale_failure_does_not_lose_original_clipboard(self) -> None: + """Newer overlapping restore must still land on the pre-first clipboard. + + If the older paste fails after generation moved on, skipping its + restore must not strand the newer thread on intermediate dictated text. + """ + obj = _make_injector() + copy_calls: list[str] = [] + nested = {"done": False} + + def run_side_effect(cmd, **kwargs): + if not nested["done"]: + nested["done"] = True + self.assertTrue(obj._inject_via_clipboard_paste("world")) + raise subprocess.CalledProcessError(1, cmd) + return MagicMock(returncode=0) + + with patch.object(obj, "_read_clipboard", side_effect=["URL", "world"]): + with patch.object( + obj, "_copy_to_clipboard", side_effect=lambda t, **kw: copy_calls.append(t) or True + ): + with patch.object(obj, "_should_copy_to_clipboard", return_value=False): + with patch.object(obj, "_should_use_terminal_paste", return_value=False): + with patch.object( + obj, + "_clipboard_paste_command", + return_value=["ydotool", "key", "ctrl+v"], + ): + with patch( + "vocalinux.text_injection.text_injector.subprocess.run", + side_effect=run_side_effect, + ): + self.assertFalse(obj._inject_via_clipboard_paste("hello")) + time.sleep(0.5) + + self.assertEqual(copy_calls[0], "hello") + self.assertEqual(copy_calls[1], "world") + self.assertEqual(copy_calls[-1], "URL") + self.assertNotIn("hello", copy_calls[2:]) + self.assertIsNone(obj._clipboard_restore_target) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_text_injector.py b/tests/test_text_injector.py index 3c13d36..30b7716 100644 --- a/tests/test_text_injector.py +++ b/tests/test_text_injector.py @@ -903,6 +903,8 @@ class TestTextInjector(unittest.TestCase): injector._state_lock = threading.Lock() injector._clipboard_restore_generation = 0 injector._clipboard_restore_target = None + injector.environment = DesktopEnvironment.WAYLAND + injector._session_environment = DesktopEnvironment.WAYLAND injector.wayland_tool = "ydotool" injector._wtype_paste_usable = False return injector @@ -1409,46 +1411,308 @@ class TestTextInjectorEdgeCases(unittest.TestCase): def test_inject_with_xdotool_xwayland_no_display(self): """Test xdotool injection in XWayland mode without DISPLAY set.""" with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland"}, clear=False): - # Temporarily remove DISPLAY env_backup = os.environ.get("DISPLAY") - if "DISPLAY" in os.environ: - del os.environ["DISPLAY"] + os.environ.pop("DISPLAY", None) try: def which_side_effect(cmd): - if cmd in ["xdotool"]: - return f"/usr/bin/{cmd}" + if cmd == "xdotool": + return "/usr/bin/xdotool" return None self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock( + returncode=0, stdout="12345", stderr="" + ) - # Create injector in WAYLAND_XDOTOOL mode - injector = TextInjector.__new__(TextInjector) - injector._state_lock = threading.Lock() + injector = TextInjector() injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL - - # Mock subprocess.run - mock_result = MagicMock() - mock_result.returncode = 0 - mock_result.stdout = "12345" - mock_result.stderr = "" - self.mock_subprocess.return_value = mock_result - - # Run injection - should set DISPLAY to :0 injector._inject_with_xdotool("test") - # Verify DISPLAY was set in the env passed to subprocess - calls = self.mock_subprocess.call_args_list - for call in calls: - if "env" in call.kwargs: - env = call.kwargs["env"] - if "type" in str(call): - self.assertEqual(env.get("DISPLAY"), ":0") + typed = [ + c + for c in self.mock_subprocess.call_args_list + if c.args and c.args[0][:2] == ["xdotool", "type"] + ] + self.assertTrue(typed, "should fall back to xdotool type without xclip") + self.assertEqual(typed[0].kwargs.get("env", {}).get("DISPLAY"), ":0") finally: - # Restore DISPLAY if env_backup is not None: os.environ["DISPLAY"] = env_backup + elif "DISPLAY" in os.environ: + del os.environ["DISPLAY"] + + def test_inject_with_xdotool_xwayland_prefers_x11_clipboard_paste(self) -> None: + """XWayland fallback must paste via xclip + xdotool ctrl+v, not `xdotool + type` (#657, and the 2026-08-17 review on PR #680). + + `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. The paste keystroke lands on an XWayland window, + which reads the X11 CLIPBOARD selection, so the copy step must use + xclip/xsel (not wl-copy) and the paste itself uses xdotool's own + `key ctrl+v` rather than ydotool: ydotool simulates Wayland-native + input, is not what an XWayland paste reads from, and per the reported + AppImage build is not even always installed. + """ + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.object(injector, "_should_use_terminal_paste", return_value=False): + injector._inject_with_xdotool("привет") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertTrue( + any(c[:2] == ["xclip", "-selection"] for c in calls), + "should copy text to the X11 CLIPBOARD selection via xclip", + ) + xclip_copy_calls = [ + c + for c in self.mock_subprocess.call_args_list + if c.args and c.args[0] == ["xclip", "-selection", "clipboard"] + ] + self.assertTrue( + xclip_copy_calls and xclip_copy_calls[0].kwargs.get("input") == "привет", + "the dictated text itself must be what xclip receives on stdin", + ) + self.assertIn( + ["xdotool", "key", "--clearmodifiers", "ctrl+v"], + calls, + "should paste with xdotool key --clearmodifiers ctrl+v", + ) + self.assertFalse( + any(c[0] == "wl-copy" for c in calls), + "must not write to the Wayland-native clipboard; an XWayland paste cannot read it", + ) + self.assertFalse( + any(c[0] == "ydotool" for c in calls), + "must not require ydotool when xclip/xsel are available", + ) + self.assertFalse( + any(c[:2] == ["xdotool", "type"] for c in calls), + "must not fall back to layout-dependent xdotool type when paste succeeds", + ) + + def test_inject_with_xdotool_xwayland_types_when_x11_clipboard_missing(self) -> None: + """No xclip/xsel: type, even if ydotool/wl-copy exist. + + ydotool+wl-copy writes the Wayland clipboard; an XWayland window + pastes the X11 CLIPBOARD. Treating ydotool rc=0 as success skips + type and leaves the wrong (or empty) selection in the field. + """ + + def which_side_effect(cmd): + if cmd in ("xdotool", "ydotool", "wl-copy"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="12345", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + injector._inject_with_xdotool("привет") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertTrue( + any(c[:2] == ["xdotool", "type"] for c in calls), + "should fall back to xdotool type when xclip/xsel are missing", + ) + self.assertFalse( + any(c[0] == "wl-copy" for c in calls), + "must not write the Wayland clipboard for an XWayland paste", + ) + self.assertFalse( + any(c[:2] == ["ydotool", "key"] for c in calls), + "must not treat ydotool ctrl+v as an XWayland paste", + ) + + def test_inject_with_xdotool_xwayland_types_when_paste_fails(self) -> None: + """xclip copy works but xdotool key fails: fall through to type.""" + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip"): + return f"/usr/bin/{cmd}" + return None + + def run_side_effect(cmd, **kwargs): + if cmd[:2] == ["xdotool", "key"]: + raise subprocess.CalledProcessError(1, cmd, stderr="paste failed") + return MagicMock(returncode=0, stdout="12345", stderr="") + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.side_effect = run_side_effect + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.object(injector, "_should_use_terminal_paste", return_value=False): + injector._inject_with_xdotool("привет") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertTrue( + any(c[:2] == ["xdotool", "type"] for c in calls), + "should fall back to xdotool type when the paste chord fails", + ) + + def test_inject_with_xdotool_xwayland_paste_sets_display(self) -> None: + """xdotool key on the paste path gets DISPLAY=:0 when it was unset.""" + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + env_backup = os.environ.get("DISPLAY") + os.environ.pop("DISPLAY", None) + try: + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.object(injector, "_should_use_terminal_paste", return_value=False): + injector._inject_with_xdotool("привет") + key_calls = [ + c + for c in self.mock_subprocess.call_args_list + if c.args and c.args[0][:2] == ["xdotool", "key"] + ] + self.assertTrue(key_calls) + self.assertEqual(key_calls[0].kwargs.get("env", {}).get("DISPLAY"), ":0") + finally: + if env_backup is not None: + os.environ["DISPLAY"] = env_backup + elif "DISPLAY" in os.environ: + del os.environ["DISPLAY"] + + def test_inject_with_xdotool_xwayland_falls_back_without_any_paste_tool(self) -> None: + """Neither xclip/xsel nor ydotool installed: keeps typing via xdotool.""" + + def which_side_effect(cmd): + if cmd == "xdotool": + return "/usr/bin/xdotool" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="12345", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + + injector._inject_with_xdotool("hello") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertTrue( + any(c[:2] == ["xdotool", "type"] for c in calls), + "should fall back to xdotool type when no clipboard-paste tool exists", + ) + + def test_inject_text_appimage_scenario_recovers_through_xclip_paste(self) -> None: + """Reproduces the #657 report through inject_text(), not the fallback + method directly (2026-08-17 review on PR #680): AppImage packaging + does not bundle ydotool, so `_try_recover_from_fallback()` must find + `which("ydotool")` false and leave `environment` at WAYLAND_XDOTOOL, + and the xdotool fallback must then paste via xclip rather than type. + """ + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="12345", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.object(injector, "_should_use_terminal_paste", return_value=False): + result = injector.inject_text("привет") + + self.assertTrue(result) + self.assertEqual( + injector.environment, + DesktopEnvironment.WAYLAND_XDOTOOL, + "recovery has no ydotool to find, so it must not claim a switch to WAYLAND", + ) + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertIn( + ["xdotool", "key", "--clearmodifiers", "ctrl+v"], + calls, + "should paste via xdotool ctrl+v against the X11 clipboard xclip wrote", + ) + self.assertFalse( + any(c[:2] == ["xdotool", "type"] for c in calls), + "must not fall through to the layout-dependent type path when the paste succeeds", + ) + + def test_inject_with_xdotool_xwayland_uses_terminal_paste_chord(self) -> None: + """XWayland xclip paste must still use Ctrl+Shift+V in terminals (#734).""" + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + with patch.object(injector, "_should_use_terminal_paste", return_value=True): + injector._inject_with_xdotool("ls") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertIn( + ["xdotool", "key", "--clearmodifiers", "ctrl+shift+v"], + calls, + "terminals must get Ctrl+Shift+V from the XWayland xdotool paste path", + ) + self.assertNotIn( + ["xdotool", "key", "--clearmodifiers", "ctrl+v"], + calls, + ) + self.assertFalse( + any(c[:2] == ["xdotool", "type"] for c in calls), + "must not fall back to xdotool type when the terminal paste succeeds", + ) + + def test_copy_to_clipboard_setting_still_prefers_wl_copy_on_xwayland(self) -> None: + """User-facing copy keeps the session clipboard; only paste is X11-only.""" + + def which_side_effect(cmd): + if cmd in ("xdotool", "xclip", "wl-copy"): + return f"/usr/bin/{cmd}" + return None + + self.mock_which.side_effect = which_side_effect + self.mock_subprocess.return_value = MagicMock(returncode=0, stdout="", stderr="") + + with patch.dict("os.environ", {"XDG_SESSION_TYPE": "wayland", "WAYLAND_DISPLAY": "w-1"}): + injector = TextInjector() + injector.environment = DesktopEnvironment.WAYLAND_XDOTOOL + injector._copy_to_clipboard("hello") + + calls = [c.args[0] for c in self.mock_subprocess.call_args_list if c.args] + self.assertTrue( + any(c[0] == "wl-copy" for c in calls), + "copy_to_clipboard setting should still prefer wl-copy on a Wayland host", + ) def test_inject_with_xdotool_releases_modifiers_without_escape(self): """The xdotool path must keep the target input focused after injection.""" diff --git a/tests/test_text_injector_ext.py b/tests/test_text_injector_ext.py index 0036589..bfc6069 100644 --- a/tests/test_text_injector_ext.py +++ b/tests/test_text_injector_ext.py @@ -80,6 +80,8 @@ def _make_injector(env) -> Any: obj._state_lock = threading.Lock() obj._clipboard_tool_health = {} obj._clipboard_timeout = 0.35 + obj._clipboard_restore_generation = 0 + obj._clipboard_restore_target = None return obj