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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user