fix(wayland): reliable clipboard copy and ydotool injection (#480)
Two independent text-injection bugs on Wayland: 1. Clipboard copy silently failed. wl-copy/xclip/xsel fork a background process that keeps owning the selection; capturing its stderr with subprocess.PIPE makes run() block on the surviving child until the clipboard is next overwritten, so the call always hits its timeout and _copy_to_clipboard reports failure even though the copy succeeded. Redirect stdout/stderr to DEVNULL so run() only waits for the short-lived foreground process. 2. ydotool scrambled text on non-US keyboard layouts. `ydotool type` emits positional evdev keycodes that get re-interpreted through the active layout (assumed US QWERTY), so dictated text came out garbled on AZERTY/QWERTZ/Dvorak (e.g. "message" -> ",essqge") and non-ASCII was dropped. Always inject via clipboard paste (Ctrl+V) for ydotool, which is layout-independent and Unicode-safe. Adds regression tests for both. Claude-Session: https://claude.ai/code/session_01EhyratrEE4vDWhriirUB9k
This commit is contained in:
@@ -331,11 +331,18 @@ class TextInjector:
|
||||
return tools
|
||||
|
||||
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
|
||||
# capturing stderr via subprocess.PIPE makes run() block until the child
|
||||
# exits (i.e. until the clipboard is next overwritten) and then time out
|
||||
# — even though the copy itself succeeded. Redirect to DEVNULL so run()
|
||||
# only waits for the short-lived foreground process.
|
||||
if tool == "wl-copy":
|
||||
subprocess.run(
|
||||
["wl-copy", text],
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
text=True,
|
||||
timeout=self._clipboard_timeout,
|
||||
)
|
||||
@@ -346,7 +353,8 @@ class TextInjector:
|
||||
["xclip", "-selection", "clipboard"],
|
||||
input=text,
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
text=True,
|
||||
timeout=self._clipboard_timeout,
|
||||
)
|
||||
@@ -357,7 +365,8 @@ class TextInjector:
|
||||
["xsel", "--clipboard", "--input"],
|
||||
input=text,
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
text=True,
|
||||
timeout=self._clipboard_timeout,
|
||||
)
|
||||
@@ -898,18 +907,19 @@ class TextInjector:
|
||||
Raises:
|
||||
subprocess.CalledProcessError: If the tool fails, with stderr captured
|
||||
"""
|
||||
# ydotool can only handle ASCII characters because it works at the
|
||||
# evdev keycode level. For non-ASCII text, use clipboard paste instead.
|
||||
if self.wayland_tool == "ydotool" and self._has_non_ascii(text):
|
||||
logger.info(
|
||||
"Text contains non-ASCII characters, using clipboard paste "
|
||||
"for ydotool (evdev keycodes are ASCII-only)"
|
||||
)
|
||||
# ydotool emits *positional* evdev keycodes, so `ydotool type` is
|
||||
# re-interpreted through the active keyboard layout, which it assumes is
|
||||
# US QWERTY. On any other layout (AZERTY, QWERTZ, Dvorak, ...) the text
|
||||
# comes out scrambled (e.g. "message" -> ",essqge" on AZERTY), and
|
||||
# non-ASCII characters are dropped entirely. Always paste via the
|
||||
# clipboard instead: Ctrl+V is layout-independent and Unicode-safe.
|
||||
if self.wayland_tool == "ydotool":
|
||||
logger.info("Using clipboard paste for ydotool (layout-independent, Unicode-safe)")
|
||||
if self._inject_via_clipboard_paste(text):
|
||||
return
|
||||
logger.warning(
|
||||
"Clipboard paste failed, falling back to ydotool type "
|
||||
"(non-ASCII characters may be dropped)"
|
||||
"(text may be scrambled on non-US layouts)"
|
||||
)
|
||||
|
||||
if self.wayland_tool == "wtype":
|
||||
|
||||
@@ -540,10 +540,16 @@ class TestTextInjector(unittest.TestCase):
|
||||
@patch("vocalinux.text_injection.text_injector.is_ibus_available", return_value=False)
|
||||
@patch("vocalinux.text_injection.text_injector.shutil.which")
|
||||
@patch("vocalinux.text_injection.text_injector.subprocess.run")
|
||||
def test_ydotool_ascii_uses_type_directly(
|
||||
def test_ydotool_ascii_also_uses_clipboard_paste(
|
||||
self, mock_run, mock_which, mock_ibus_avail, mock_ibus_active
|
||||
):
|
||||
"""Test that ydotool still uses type for plain ASCII text."""
|
||||
"""ydotool always pastes via the clipboard, even for plain ASCII.
|
||||
|
||||
`ydotool type` emits positional evdev keycodes that get re-interpreted
|
||||
through the active keyboard layout (assumed US QWERTY), so typing ASCII
|
||||
is scrambled on non-US layouts (e.g. AZERTY). Clipboard paste (Ctrl+V)
|
||||
is layout-independent, so it is used unconditionally for ydotool.
|
||||
"""
|
||||
mock_which.side_effect = lambda x: x in ("ydotool", "wl-copy")
|
||||
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||
|
||||
@@ -557,12 +563,16 @@ class TestTextInjector(unittest.TestCase):
|
||||
|
||||
calls = [c.args[0] for c in mock_run.call_args_list if c.args]
|
||||
self.assertTrue(
|
||||
any(c[:2] == ["ydotool", "type"] for c in calls),
|
||||
"Should use ydotool type for ASCII text",
|
||||
any(c[0] == "wl-copy" for c in calls),
|
||||
"Should copy ASCII text to the clipboard",
|
||||
)
|
||||
self.assertTrue(
|
||||
any(c[:2] == ["ydotool", "key"] for c in calls),
|
||||
"Should paste with ydotool key (Ctrl+V)",
|
||||
)
|
||||
self.assertFalse(
|
||||
any(c[0] == "wl-copy" for c in calls),
|
||||
"Should NOT invoke clipboard for ASCII text",
|
||||
any(c[:2] == ["ydotool", "type"] for c in calls),
|
||||
"Should NOT use layout-dependent ydotool type",
|
||||
)
|
||||
|
||||
@patch("vocalinux.text_injection.text_injector.is_ibus_active_input_method", return_value=False)
|
||||
@@ -627,6 +637,27 @@ class TestTextInjector(unittest.TestCase):
|
||||
has_xsel = any(c[0] == "xsel" for c in calls)
|
||||
self.assertTrue(has_xsel, "Should use xsel as fallback")
|
||||
|
||||
@patch("vocalinux.text_injection.text_injector.subprocess.run")
|
||||
def test_clipboard_command_does_not_capture_pipe(self, mock_run):
|
||||
"""_run_clipboard_command must not capture the tool's stderr via a pipe.
|
||||
|
||||
wl-copy/xclip/xsel fork a background process that keeps owning the
|
||||
selection; if stderr is captured with subprocess.PIPE the call blocks on
|
||||
the surviving child and times out even though the copy succeeded.
|
||||
Redirecting to DEVNULL avoids that hang (regression guard).
|
||||
"""
|
||||
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||
injector = TextInjector.__new__(TextInjector)
|
||||
injector._clipboard_timeout = 0.35
|
||||
|
||||
for tool in ("wl-copy", "xclip", "xsel"):
|
||||
mock_run.reset_mock()
|
||||
self.assertTrue(injector._run_clipboard_command(tool, "café"))
|
||||
_, kwargs = mock_run.call_args
|
||||
self.assertEqual(kwargs.get("stdout"), subprocess.DEVNULL)
|
||||
self.assertEqual(kwargs.get("stderr"), subprocess.DEVNULL)
|
||||
self.assertNotEqual(kwargs.get("stderr"), subprocess.PIPE)
|
||||
|
||||
@patch("vocalinux.text_injection.text_injector.is_ibus_active_input_method", return_value=False)
|
||||
@patch("vocalinux.text_injection.text_injector.is_ibus_available", return_value=False)
|
||||
@patch("vocalinux.text_injection.text_injector.shutil.which")
|
||||
|
||||
Reference in New Issue
Block a user