From 330e1d82373ab64258e1c24335b7e7cbda5d065a Mon Sep 17 00:00:00 2001 From: Benjamin Eisele <163771687+eiseleb47@users.noreply.github.com> Date: Wed, 2 Sep 2026 08:54:15 +0000 Subject: [PATCH] fix(actions): send real BackSpace key events for "delete that" (#714) `_handle_delete_last` built its deletion as `"\b" * len(text)` and passed it to `inject_text`, treating U+0008 as typed input. No backend delivers that as a keypress: - `ydotool type` has no keymap entry for U+0008, so it types nothing. - IBus `commit_text()` commits it as a literal control character, which the client inserts or drops rather than treating as a deletion. Only an actual key event deletes, so the "delete that" voice command did nothing on those paths while still reporting success. Add `press_backspace(count)`, which sends `count` real BackSpace events using the tool that fits the environment: `xdotool key --repeat` on X11 and the xdotool-backed Wayland path, `wtype -k BackSpace` or the ydotool keycode pair `14:1 14:0` otherwise. WAYLAND_IBUS resolves a virtual-keyboard tool of its own, since IBus cannot send key events. `inject_text` is untouched; only the delete_last action changes. The existing delete_last tests asserted the U+0008 string was injected, so they now pin the new contract instead: press_backspace called once with the character count, and inject_text not called at all. --- src/vocalinux/text_injection/text_injector.py | 109 +++++++++++++++ src/vocalinux/ui/action_handler.py | 7 +- tests/test_action_handler.py | 11 +- tests/test_text_injector_ext.py | 125 ++++++++++++++++++ 4 files changed, 243 insertions(+), 9 deletions(-) diff --git a/src/vocalinux/text_injection/text_injector.py b/src/vocalinux/text_injection/text_injector.py index a8bc71b..8769519 100644 --- a/src/vocalinux/text_injection/text_injector.py +++ b/src/vocalinux/text_injection/text_injector.py @@ -1977,6 +1977,115 @@ class TextInjector: raise ValueError("no key found") return steps + def press_backspace(self, count: int) -> bool: + """Send ``count`` real BackSpace key events. + + Deleting text cannot be done by injecting U+0008 characters: ydotool has + no keymap entry for it (so it types nothing) and IBus ``commit_text()`` + commits it as a literal control character. Only actual key events delete. + + Args: + count: Number of backspaces to send. + + Returns: + True if the presses were delivered, False otherwise. + """ + if count <= 0: + return True + + logger.debug(f"Sending {count} backspace key event(s)") + + if ( + self.environment == DesktopEnvironment.X11 + or self.environment == DesktopEnvironment.WAYLAND_XDOTOOL + or self.environment == DesktopEnvironment.X11_IBUS + ): + env = os.environ.copy() + if self.environment == DesktopEnvironment.WAYLAND_XDOTOOL: + env["GDK_BACKEND"] = "x11" + env["QT_QPA_PLATFORM"] = "xcb" + if not env.get("DISPLAY"): + env["DISPLAY"] = ":0" + try: + # --clearmodifiers already neutralises a held modifier here, so + # this path needs no _wait_for_modifiers_released(). + subprocess.run( + ["xdotool", "key", "--clearmodifiers", "--repeat", str(count), "BackSpace"], + env=host_env(env), + check=True, + stderr=subprocess.PIPE, + text=True, + timeout=max(3, count * 0.05), + ) + return True + except ( + subprocess.CalledProcessError, + subprocess.TimeoutExpired, + FileNotFoundError, + ) as e: + logger.error(f"xdotool backspace error: {e}") + return False + + # Wayland (including WAYLAND_IBUS -- IBus cannot send key events). + # A held PTT modifier would turn every BackSpace into Ctrl+BackSpace + # (delete-word), so wait it out before resolving and sending. + self._wait_for_modifiers_released() + tool = self._resolve_wayland_key_tool() + if not tool: + logger.error("No virtual-keyboard tool available to send backspaces") + return False + + if tool == "ydotool" and not self._ensure_ydotoold(): + # Checked on every call, not just when the tool is first resolved: + # wayland_tool is cached from startup, so a daemon that died since + # would otherwise never be restarted for this path alone. Both other + # ydotool call sites warn and continue the same way. + logger.warning("ydotoold not ready before backspace injection") + + if tool == "wtype": + cmd = ["wtype"] + ["-k", "BackSpace"] * count + timeout = max(3, count * 0.01) + elif self._ydotool_uses_legacy_named_keys(): + # 0.1.x: N named tokens in one call. Its per-call delay budget is + # spread across all events, so this stays flat regardless of count + # -- unlike --repeat, which re-runs the sequence at ~100ms each. + cmd = ["ydotool", "key"] + [self._ydotool_legacy_token([], "backspace")] * count + timeout = 5 + else: + # 1.x sleeps --key-delay after EVERY token (12ms by default), so a + # 400-character deletion would otherwise take ~9.6s. Match the delay + # `ydotool type` already uses, and scale the budget with the count. + code = self._KEYCODES["backspace"] + delay = os.environ.get("VOCALINUX_YDOTOOL_KEY_DELAY", "2") + cmd = ["ydotool", "key", "--key-delay", delay] + [f"{code}:1", f"{code}:0"] * count + timeout = max(3, count * 2 * self._key_delay_seconds(delay) * 3) + + try: + subprocess.run( + cmd, + check=True, + stderr=subprocess.PIPE, + text=True, + timeout=timeout, + env=host_env(), + ) + return True + except ( + subprocess.CalledProcessError, + subprocess.TimeoutExpired, + FileNotFoundError, + ) as e: + logger.error(f"{tool} backspace error: {e}") + return False + + @staticmethod + def _key_delay_seconds(raw: str) -> float: + """Parse a ydotool key-delay in ms, falling back to its 12ms default.""" + try: + return max(0.0, int(raw)) / 1000 + except (TypeError, ValueError): + return 0.012 + def _log_current_window_info(self): """Log information about the current window/application for debugging.""" try: diff --git a/src/vocalinux/ui/action_handler.py b/src/vocalinux/ui/action_handler.py index d6fa77f..fc3e477 100644 --- a/src/vocalinux/ui/action_handler.py +++ b/src/vocalinux/ui/action_handler.py @@ -98,9 +98,10 @@ class ActionHandler: logger.debug("No text to delete") return True - # Send backspace keys for each character in the last injected text - backspaces = "\b" * len(self.last_injected_text) - success = self.text_injector.inject_text(backspaces) + # Send real BackSpace key events, one per character. Injecting U+0008 as + # text does nothing: ydotool has no keymap entry for it and IBus commits + # it as a literal control character. + success = self.text_injector.press_backspace(len(self.last_injected_text)) if success: logger.debug(f"Deleted {len(self.last_injected_text)} characters") diff --git a/tests/test_action_handler.py b/tests/test_action_handler.py index caeca06..bf7075d 100644 --- a/tests/test_action_handler.py +++ b/tests/test_action_handler.py @@ -53,10 +53,9 @@ class TestActionHandler(unittest.TestCase): result = self.handler.handle_action("delete_last") self.assertTrue(result) - self.mock_text_injector.inject_text.assert_called_once() - # Should send 5 backspaces for "hello" - call_args = self.mock_text_injector.inject_text.call_args[0][0] - self.assertEqual(len(call_args), 5) # 5 backspaces + # Real BackSpace key events, not injected U+0008 text. + self.mock_text_injector.press_backspace.assert_called_once_with(5) + self.mock_text_injector.inject_text.assert_not_called() self.assertEqual(self.handler.last_injected_text, "") def test_handle_undo(self): @@ -131,9 +130,9 @@ class TestActionHandler(unittest.TestCase): self.assertFalse(result) def test_delete_last_failure(self): - """Test delete_last when inject_text fails.""" + """Test delete_last when the backspace presses fail.""" self.handler.set_last_injected_text("test") - self.mock_text_injector.inject_text.return_value = False + self.mock_text_injector.press_backspace.return_value = False result = self.handler.handle_action("delete_last") diff --git a/tests/test_text_injector_ext.py b/tests/test_text_injector_ext.py index 085644e..0036589 100644 --- a/tests/test_text_injector_ext.py +++ b/tests/test_text_injector_ext.py @@ -931,6 +931,131 @@ class TestInjectKeyboardShortcutRouting(unittest.TestCase): self.assertEqual(obj.wayland_tool, "ydotool") +def _make_backspace_injector(env_name, tool=None, legacy=False): + """Injector wired for press_backspace, with the dialect and daemon pinned.""" + from vocalinux.text_injection.text_injector import DesktopEnvironment + + obj = _make_injector(getattr(DesktopEnvironment, env_name)) + if tool is not None: + obj.wayland_tool = tool + obj._ydotool_legacy_named_keys = legacy + obj._ensure_ydotoold = MagicMock(return_value=True) + obj._wait_for_modifiers_released = MagicMock() + return obj + + +class TestPressBackspace(unittest.TestCase): + def test_zero_is_noop(self): + obj = _make_backspace_injector("WAYLAND") + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(0)) + mock_run.assert_not_called() + + def test_wtype_repeats_key_events(self): + obj = _make_backspace_injector("WAYLAND", tool="wtype") + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(3)) + self.assertEqual( + mock_run.call_args[0][0], + ["wtype", "-k", "BackSpace", "-k", "BackSpace", "-k", "BackSpace"], + ) + + def test_ydotool_v1_uses_keycode_14(self): + obj = _make_backspace_injector("WAYLAND", tool="ydotool", legacy=False) + with patch.dict(os.environ, {"VOCALINUX_YDOTOOL_KEY_DELAY": "2"}): + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(2)) + self.assertEqual( + mock_run.call_args[0][0], + ["ydotool", "key", "--key-delay", "2", "14:1", "14:0", "14:1", "14:0"], + ) + + def test_ydotool_legacy_uses_named_backspace(self): + """1.x keycodes type digit garbage on 0.1.x, which exits 0 regardless.""" + obj = _make_backspace_injector("WAYLAND", tool="ydotool", legacy=True) + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(2)) + self.assertEqual( + mock_run.call_args[0][0], + ["ydotool", "key", "backspace", "backspace"], + ) + + def test_x11_uses_xdotool_repeat_without_waiting(self): + """--clearmodifiers covers held modifiers, so no wait on this path.""" + obj = _make_backspace_injector("X11") + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(5)) + self.assertEqual( + mock_run.call_args[0][0], + ["xdotool", "key", "--clearmodifiers", "--repeat", "5", "BackSpace"], + ) + obj._wait_for_modifiers_released.assert_not_called() + kwargs = mock_run.call_args[1] + self.assertIn("env", kwargs) + self.assertGreaterEqual(kwargs["timeout"], 3) + + def test_wayland_ibus_falls_back_to_virtual_keyboard(self): + """IBus cannot send key events, so deletion must use wtype/ydotool.""" + obj = _make_backspace_injector("WAYLAND_IBUS") + which = lambda c: "/usr/bin/wtype" if c == "wtype" else None # noqa: E731 + with patch("subprocess.run") as mock_run: + with patch("shutil.which", side_effect=which): + self.assertTrue(obj.press_backspace(1)) + self.assertEqual(mock_run.call_args[0][0], ["wtype", "-k", "BackSpace"]) + + def test_prefers_ydotool_when_both_tools_are_present(self): + """_check_dependencies prefers the uinput helper; match that order.""" + obj = _make_backspace_injector("WAYLAND_IBUS", legacy=False) + with patch("subprocess.run") as mock_run: + with patch("shutil.which", return_value="/usr/bin/x"): + self.assertTrue(obj.press_backspace(1)) + self.assertEqual(obj.wayland_tool, "ydotool") + self.assertEqual(mock_run.call_args[0][0][:2], ["ydotool", "key"]) + + def test_waits_for_modifiers_before_resolving_the_tool(self): + """A held PTT modifier turns BackSpace into Ctrl+BackSpace.""" + obj = _make_backspace_injector("WAYLAND", tool="wtype") + order = [] + obj._wait_for_modifiers_released = MagicMock(side_effect=lambda: order.append("wait")) + with patch("subprocess.run", side_effect=lambda *a, **k: order.append("run")): + self.assertTrue(obj.press_backspace(1)) + self.assertEqual(order, ["wait", "run"]) + + def test_ydotool_rechecks_the_daemon_even_when_the_tool_is_cached(self): + """wayland_tool is cached at startup, so a daemon that died since would + never be restarted if the check only ran on a cold resolve.""" + obj = _make_backspace_injector("WAYLAND", tool="ydotool", legacy=False) + with patch("subprocess.run"): + self.assertTrue(obj.press_backspace(1)) + obj._ensure_ydotoold.assert_called_once() + + def test_ydotool_continues_when_daemon_not_ready(self): + """0.1.x often ships no daemon at all, so this must not abort.""" + obj = _make_backspace_injector("WAYLAND", tool="ydotool", legacy=False) + obj._ensure_ydotoold = MagicMock(return_value=False) + with patch("subprocess.run") as mock_run: + self.assertTrue(obj.press_backspace(1)) + self.assertTrue(mock_run.called) + + def test_wtype_does_not_touch_the_ydotool_daemon(self): + obj = _make_backspace_injector("WAYLAND", tool="wtype") + with patch("subprocess.run"): + self.assertTrue(obj.press_backspace(1)) + obj._ensure_ydotoold.assert_not_called() + + def test_timeout_returns_false_rather_than_raising(self): + obj = _make_backspace_injector("WAYLAND", tool="wtype") + with patch("subprocess.run", side_effect=subprocess.TimeoutExpired("wtype", 3)): + self.assertFalse(obj.press_backspace(2)) + + def test_no_tool_available_injects_nothing(self): + obj = _make_backspace_injector("WAYLAND_IBUS") + with patch("subprocess.run") as mock_run: + with patch("shutil.which", return_value=None): + self.assertFalse(obj.press_backspace(3)) + mock_run.assert_not_called() + + class TestCopyToClipboard(unittest.TestCase): def test_copy_success(self): from vocalinux.text_injection.text_injector import DesktopEnvironment