From e100af7bcde424012ee4327063d9b7692d482e9e Mon Sep 17 00:00:00 2001 From: soubarnak Date: Fri, 6 Nov 2026 09:21:13 +0530 Subject: [PATCH] fix(injection): align fallback ladder to portal>wtype>ydotool>xdotool>manual Ticket #31 review found startup and demote paths preferring ydotool over wtype, with a KDE ydotool-first exception, contradicting the decided order in spec #16 (portal first where a backend implements it, then direct keysym typing, then the virtual-input tool, then XWayland-only injection, then manual paste). - Auto-selection is now portal, wtype, ready ydotool, daemonless ydotool, xdotool on every desktop including KDE; the __init__ wtype probe validates wtype and demotes on compositor rejection. - Portal-deny demotion, IBus-fallback switch, XWayland recovery, and a new wtype runtime-failure retry all follow the same rung order with partial-type accounting preserved. - Ladder suite grows to 14 tests pinning KDE portal selection, wtype-before-ydotool selection, and each demotion trigger. - USER_GUIDE states the full rung order and portal-first on KDE. --- docs/USER_GUIDE.md | 19 +- src/vocalinux/text_injection/text_injector.py | 186 +++++++++++------- tests/test_injection_ladder.py | 131 +++++++++++- tests/test_text_injector.py | 17 +- tests/test_text_injector_ext.py | 9 +- 5 files changed, 265 insertions(+), 97 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 42660b2..da62c57 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -383,10 +383,15 @@ as a niri recommendation — there is nothing for it to talk to. IBus stays excluded on niri as well: it would report the text as delivered while native windows silently receive nothing. -**GNOME / KDE Plasma.** The portal leads where a backend implements it. -Nothing asks at startup: the handshake stays lazy and the compositor asks -once, for keyboard access only, at the first actual dictation. A granted -session keeps its restore token so later launches skip the prompt where the -desktop supports persistence. Denying closes that session and is not retried -within the run; the pending dictation is still typed through the next -fallback rung, so saying no costs a fallback rather than the dictation. +**GNOME / KDE Plasma.** The portal leads on both: GNOME and KDE Plasma +implement `org.freedesktop.portal.RemoteDesktop`, so it is the first rung +on either desktop. After it the order is direct keysym typing (`wtype`), +then the virtual-input tool (`ydotool`, needs a ready `ydotoold`), then +XWayland-only typing (`xdotool`), then clipboard copy with manual paste as +the last resort. Nothing asks at startup: the handshake stays lazy and the +compositor asks once, for keyboard access only, at the first actual +dictation. A granted session keeps its restore token so later launches +skip the prompt where the desktop supports persistence. Denying closes +that session and is not retried within the run; the pending dictation is +still typed through direct typing, so saying no costs a fallback rather +than the dictation. diff --git a/src/vocalinux/text_injection/text_injector.py b/src/vocalinux/text_injection/text_injector.py index fc9f178..ce534bf 100644 --- a/src/vocalinux/text_injection/text_injector.py +++ b/src/vocalinux/text_injection/text_injector.py @@ -1007,16 +1007,17 @@ class TextInjector: if ydotool_available: _warn_if_ydotool_globally_enabled() - # Prefer ydotool when the daemon is (or can be) ready. Flatpak ships - # ydotool for native Wayland typing; wtype needs a Wayland socket. - # The RemoteDesktop portal sits ahead of both in autodetection: it - # is the only injection path Wayland sanctions, works inside the - # Flatpak sandbox without /dev/uinput, and reaches native clients - # on compositors with no input-method-v2 bridge. KDE is the - # exception: KWin's portal scrambles letter case (#911), so on - # Plasma a present ydotool goes first -- the ordering from before - # the portal backend existed. - kde_ydotool_first = ydotool_available and _is_kde_plasma_session() + # Decided ladder (spec #16, ticket #31): the RemoteDesktop portal + # sits ahead of both typing tools in autodetection -- it is the + # only injection path Wayland sanctions, works inside the Flatpak + # sandbox without /dev/uinput, and reaches native clients on + # compositors with no input-method-v2 bridge. Then direct keysym + # typing (wtype), then the virtual-input tool (ydotool, which + # needs a ready ydotoold), then XWayland-only xdotool, then manual + # paste. A daemonless ydotool must not outrank a working portal: + # it may fail at injection time (no /dev/uinput), while the + # portal delivers. The __init__ wtype probe demotes wtype to + # ydotool when the compositor rejects the virtual keyboard. if forced == "portal" and portal_available: self._select_portal_backend( "%s=portal: using RemoteDesktop portal for Wayland injection", @@ -1029,30 +1030,22 @@ class TextInjector: self._ensure_ydotoold() self.wayland_tool = "ydotool" logger.info("%s=ydotool: using ydotool for Wayland injection", pin_source) - elif portal_available and not kde_ydotool_first: + elif portal_available: self._select_portal_backend( "Using the RemoteDesktop portal for Wayland text injection" ) + elif wtype_available: + self.wayland_tool = "wtype" + logger.info("Using wtype for Wayland text injection") elif ydotool_available and self._ensure_ydotoold(): self.wayland_tool = "ydotool" logger.info("Using ydotool for Wayland text injection") - elif portal_available: - # Reached only on KDE after ydotool proved unusable; on other - # desktops the earlier portal branch already fired. A daemonless - # ydotool must not outrank a working portal: it may fail at - # injection time (no /dev/uinput), while the portal delivers. - self._select_portal_backend( - "Using the RemoteDesktop portal for Wayland text injection" - ) - elif ydotool_available and not wtype_available: + elif ydotool_available: self.wayland_tool = "ydotool" logger.warning( "ydotoold not ready; using ydotool without daemon " "(may fail or have latency/permission issues)" ) - elif wtype_available: - self.wayland_tool = "wtype" - logger.info(f"Using {self.wayland_tool} for Wayland text injection") elif xdotool_available: # Fallback to xdotool with XWayland self.environment = DesktopEnvironment.WAYLAND_XDOTOOL @@ -1201,16 +1194,21 @@ class TextInjector: def _demote_portal_backend(self) -> bool: """Drop the portal for this run and pick the next Wayland tool. - Mirrors the startup preference: ydotool with a ready daemon, then - ydotool alone when wtype is absent, then wtype, then the XWayland - fallback. Returns False when nothing else exists, leaving the - clipboard fallback to report the failure. In a Flatpak, keep the - portal and reset its session when ydotoold is unavailable so the next - attempt can create a fresh session. + Mirrors the startup preference (spec #16, ticket #31): direct keysym + typing (wtype), then ydotool with a ready daemon, then daemonless + ydotool, then the XWayland fallback. Returns False when nothing else + exists, leaving the clipboard fallback to report the failure. In a + Flatpak, keep the portal and reset its session when no typing tool + is usable so the next attempt can create a fresh session. """ flatpak = bool(os.environ.get("FLATPAK_ID")) ydotool_available = shutil.which("ydotool") is not None - if flatpak and (not ydotool_available or not self._ensure_ydotoold()): + wtype_available = shutil.which("wtype") is not None + if ( + flatpak + and not wtype_available + and (not ydotool_available or not self._ensure_ydotoold()) + ): portal = getattr(self, "_portal", None) if portal is not None: try: @@ -1220,7 +1218,7 @@ class TextInjector: logger.warning( "RemoteDesktop portal failed; keeping it for the next attempt " "(the session will be recreated on the next attempt; no working " - "ydotool daemon is available in the Flatpak)" + "typing tool is available in the Flatpak)" ) return False @@ -1230,17 +1228,16 @@ class TextInjector: self.wayland_tool = None if flatpak: - tool = "ydotool" + tool = "wtype" if wtype_available else "ydotool" else: - wtype_available = shutil.which("wtype") is not None xdotool_available = shutil.which("xdotool") is not None tool = None - if ydotool_available and self._ensure_ydotoold(): - tool = "ydotool" - elif ydotool_available and not wtype_available: - tool = "ydotool" - elif wtype_available: + if wtype_available: tool = "wtype" + elif ydotool_available and self._ensure_ydotoold(): + tool = "ydotool" + elif ydotool_available: + tool = "ydotool" elif xdotool_available: with self._state_lock: self.environment = DesktopEnvironment.WAYLAND_XDOTOOL @@ -1415,6 +1412,15 @@ class TextInjector: wtype_available = shutil.which("wtype") is not None xdotool_available = shutil.which("xdotool") is not None + # Decided ladder order (spec #16, ticket #31): direct keysym + # typing first, then the virtual-input tool, then XWayland. + if wtype_available: + with self._state_lock: + self.wayland_tool = "wtype" + self.environment = DesktopEnvironment.WAYLAND + logger.warning("IBus injection failed, switching to Wayland wtype fallback") + return True + if ydotool_available: try: subprocess.run( @@ -1436,13 +1442,6 @@ class TextInjector: ): logger.debug("ydotool fallback unavailable (daemon not running)") - if wtype_available: - with self._state_lock: - self.wayland_tool = "wtype" - self.environment = DesktopEnvironment.WAYLAND - logger.warning("IBus injection failed, switching to Wayland wtype fallback") - return True - if xdotool_available: with self._state_lock: self.environment = DesktopEnvironment.WAYLAND_XDOTOOL @@ -1451,7 +1450,7 @@ class TextInjector: logger.error( "IBus fallback failed: no Wayland text injection tools available " - "(ydotool, wtype, xdotool)" + "(wtype, ydotool, xdotool)" ) return False @@ -1502,6 +1501,21 @@ class TextInjector: logger.info("Checking for better Wayland text injection tools...") + # Decided ladder order (spec #16, ticket #31): direct keysym typing + # first, then the virtual-input tool. + # Check for wtype with compositor support + if shutil.which("wtype"): + try: + result = self._probe_wtype_support() + error_output = result.stderr.lower() + if "compositor does not support" not in error_output and result.returncode == 0: + self.wayland_tool = "wtype" + self.environment = DesktopEnvironment.WAYLAND + logger.info("Recovered to wtype - compositor now supports virtual keyboard") + return True + except Exception as e: + logger.debug(f"Error testing wtype: {e}") + # Check for ydotool with daemon running if shutil.which("ydotool"): try: @@ -1519,19 +1533,6 @@ class TextInjector: except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError): logger.debug("ydotool available but daemon not running") - # Check for wtype with compositor support - if shutil.which("wtype"): - try: - result = self._probe_wtype_support() - error_output = result.stderr.lower() - if "compositor does not support" not in error_output and result.returncode == 0: - self.wayland_tool = "wtype" - self.environment = DesktopEnvironment.WAYLAND - logger.info("Recovered to wtype - compositor now supports virtual keyboard") - return True - except Exception as e: - logger.debug(f"Error testing wtype: {e}") - logger.debug("No better tools available, continuing with xdotool fallback") return False @@ -1794,31 +1795,66 @@ class TextInjector: "compositor does not support" in str(e).lower() or "compositor does not support" in stderr_msg.lower() ) - logger.warning( - f"Wayland tool failed: {e}. stderr: {stderr_msg}. Falling back to xdotool" - ) + # A wtype failure at injection time demotes to the next + # ladder rung (ydotool) before the XWayland-only path, + # mirroring the startup order (spec #16, ticket #31). + ydotool_retry_succeeded = False if ( - unsupported_wayland - and current_env == DesktopEnvironment.WAYLAND - and _is_kde_plasma_session() + self.wayland_tool == "wtype" + and shutil.which("ydotool") + and self._ensure_ydotoold() ): logger.warning( - "KDE Plasma Wayland rejected virtual keyboard injection. " - f"{_kde_wayland_ibus_hint()}" - ) - if unsupported_wayland and shutil.which("xdotool"): - logger.info( - "Switching to XWayland fallback - will re-check for better tools" + f"Wayland tool failed: {e}. stderr: {stderr_msg}. " + "Falling back to ydotool" ) with self._state_lock: - self.environment = DesktopEnvironment.WAYLAND_XDOTOOL + self.wayland_tool = "ydotool" try: - self._inject_with_xdotool(remaining) + self._inject_with_wayland_tool(remaining) except _PartiallyTyped as nested: # Keep the count relative to the original text. raise _PartiallyTyped(typed + nested.typed, nested) from nested - else: - raise + except subprocess.CalledProcessError as retry_error: + e = retry_error + stderr_msg = ( + retry_error.stderr.strip() + if retry_error.stderr + else "No stderr output" + ) + unsupported_wayland = ( + "compositor does not support" in str(retry_error).lower() + or "compositor does not support" in stderr_msg.lower() + ) + else: + ydotool_retry_succeeded = True + if not ydotool_retry_succeeded: + logger.warning( + f"Wayland tool failed: {e}. stderr: {stderr_msg}. " + "Falling back to xdotool" + ) + if ( + unsupported_wayland + and current_env == DesktopEnvironment.WAYLAND + and _is_kde_plasma_session() + ): + logger.warning( + "KDE Plasma Wayland rejected virtual keyboard injection. " + f"{_kde_wayland_ibus_hint()}" + ) + if unsupported_wayland and shutil.which("xdotool"): + logger.info( + "Switching to XWayland fallback - will re-check for better tools" + ) + with self._state_lock: + self.environment = DesktopEnvironment.WAYLAND_XDOTOOL + try: + self._inject_with_xdotool(remaining) + except _PartiallyTyped as nested: + # Keep the count relative to the original text. + raise _PartiallyTyped(typed + nested.typed, nested) from nested + else: + raise logger.info("Text injection completed successfully") if self._should_copy_to_clipboard(): diff --git a/tests/test_injection_ladder.py b/tests/test_injection_ladder.py index 3b3f5ef..2415a3b 100644 --- a/tests/test_injection_ladder.py +++ b/tests/test_injection_ladder.py @@ -20,11 +20,12 @@ Seams under test (public boundaries, no internals): terminal losing its Shift chord, a held push-to-talk modifier corrupting the paste. -What this file does NOT pin: the wtype-vs-ydotool preference when both are -installed. That order is an implementation choice with Flatpak/KDE -constraints behind it; the ladder contract here is that the selected -direct-typing tool delivers without involving the portal or IBus on niri -and without losing the clipboard contents. +The decided rung order pinned here (spec #16, ticket #31) is: the +RemoteDesktop portal first where a backend implements it, then direct +keysym typing (wtype), then the virtual-input tool (ydotool, which needs +a ready ydotoold), then XWayland-only injection (xdotool), then manual +paste. KDE follows the same order: a working portal leads there too, and +a denial demotes to direct typing for the run. """ import subprocess @@ -201,6 +202,22 @@ class TestPortalFirstWhereImplemented: assert injector.wayland_tool == "portal" assert injector.environment == DesktopEnvironment.WAYLAND + def test_kde_selects_portal_where_backend_exists(self, monkeypatch: pytest.MonkeyPatch) -> None: + """KDE follows the same order: a working portal leads (#31). + + A present ydotool must not outrank an available portal, even with + its daemon ready; the portal denial path below is what demotes. + """ + injector = _selecting_injector( + monkeypatch, + desktop="KDE", + tools={"wtype": "/usr/bin/wtype", "ydotool": "/usr/bin/ydotool"}, + portal_available=True, + ydotool_ready=True, + ) + assert injector.wayland_tool == "portal" + assert injector.environment == DesktopEnvironment.WAYLAND + def test_portal_deny_falls_back_without_losing_dictation( self, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -231,6 +248,110 @@ class TestPortalFirstWhereImplemented: assert runs == [["wtype", "--", "hello"]] +class TestDirectTypingBeforeVirtualInput: + """Without a portal, direct keysym typing outranks the virtual-input tool.""" + + def test_no_portal_prefers_wtype_when_both_installed( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The decided second rung is wtype even with ydotool ready (#31).""" + injector = _selecting_injector( + monkeypatch, + desktop="GNOME", + tools={"wtype": "/usr/bin/wtype", "ydotool": "/usr/bin/ydotool"}, + portal_available=False, + ydotool_ready=True, + ) + assert injector.wayland_tool == "wtype" + assert injector.environment == DesktopEnvironment.WAYLAND + + def test_no_portal_no_wtype_uses_ready_ydotool(self, monkeypatch: pytest.MonkeyPatch) -> None: + injector = _selecting_injector( + monkeypatch, + desktop="GNOME", + tools={"ydotool": "/usr/bin/ydotool"}, + portal_available=False, + ydotool_ready=True, + ) + assert injector.wayland_tool == "ydotool" + + def test_no_portal_daemonless_ydotool_without_wtype_still_types( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A daemonless ydotool is picked with a warning when wtype is absent.""" + injector = _selecting_injector( + monkeypatch, + desktop="GNOME", + tools={"ydotool": "/usr/bin/ydotool"}, + portal_available=False, + ydotool_ready=False, + ) + assert injector.wayland_tool == "ydotool" + + def test_portal_deny_demotes_to_wtype_when_both_installed( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A denied portal demotes to direct typing, not the virtual tool.""" + injector = _bare_wayland_injector( + environment=DesktopEnvironment.WAYLAND, wayland_tool="portal" + ) + portal = MagicMock() + portal.inject_text.side_effect = RemoteDesktopPortalError("denied by user", delivered=0) + injector._portal = portal + monkeypatch.setattr( + "shutil.which", + lambda cmd: f"/usr/bin/{cmd}" if cmd in ("wtype", "ydotool") else None, + ) + monkeypatch.setattr(TextInjector, "_ensure_ydotoold", lambda self: True) + runs = [] + + def fake_run(cmd: Any, **kwargs: Any) -> MagicMock: + runs.append(cmd) + return MagicMock(returncode=0, stderr="") + + monkeypatch.setattr("subprocess.run", fake_run) + injector._inject_with_wayland_tool("hello") + portal.inject_text.assert_called_once_with("hello") + assert injector.wayland_tool == "wtype" + assert runs == [["wtype", "--", "hello"]] + + def test_wtype_runtime_failure_demotes_to_ydotool( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A wtype failure at injection time falls to ydotool whole (#31).""" + injector = _bare_wayland_injector( + environment=DesktopEnvironment.WAYLAND, wayland_tool="wtype" + ) + injector._session_environment = DesktopEnvironment.WAYLAND + attempts = [] + pastes = [] + + def fake_run(cmd: Any, **kwargs: Any) -> MagicMock: + attempts.append(cmd) + if cmd[0] == "wtype": + raise subprocess.CalledProcessError(1, cmd, stderr="boom") + return MagicMock(returncode=0, stderr="") + + monkeypatch.setattr("subprocess.run", fake_run) + monkeypatch.setattr( + "shutil.which", + lambda cmd: f"/usr/bin/{cmd}" if cmd in ("wtype", "ydotool") else None, + ) + monkeypatch.setattr(TextInjector, "_ensure_ydotoold", lambda self: True) + with ( + patch.object( + injector, + "_inject_via_clipboard_paste", + side_effect=lambda text, **kwargs: pastes.append(text) or True, + ), + patch.object(injector, "_should_copy_to_clipboard", return_value=False), + ): + assert injector.inject_text("hi") is True + assert injector.wayland_tool == "ydotool" + assert [cmd for cmd in attempts if cmd[0] == "wtype"] == [["wtype", "--", "hi"]] + assert pastes == ["hi"] + + class TestTerminalAndModifierGating: """Terminals get the terminal chord; held modifiers wait first.""" diff --git a/tests/test_text_injector.py b/tests/test_text_injector.py index fc0fce9..027cee0 100644 --- a/tests/test_text_injector.py +++ b/tests/test_text_injector.py @@ -3253,10 +3253,15 @@ class TestCompositorIBusBridging(unittest.TestCase): @patch("vocalinux.text_injection.text_injector.is_ibus_available", return_value=False) @patch("vocalinux.text_injection.text_injector.shutil.which") - def test_kde_auto_prefers_ydotool_over_portal( + def test_kde_auto_prefers_portal_over_ydotool( self, mock_which: MagicMock, _mock_ibus: MagicMock ) -> None: - """KWin's portal scrambles letter case: on KDE, ydotool goes first (#911).""" + """KDE follows the decided ladder: a working portal leads (spec #16, #31). + + This supersedes the #911 ydotool-first order: the portal is the only + injection path Wayland sanctions, and a denial demotes to direct + typing for the run instead of losing the dictation. + """ mock_which.side_effect = lambda cmd: { "ydotool": "/usr/bin/ydotool", "ydotoold": "/usr/bin/ydotoold", @@ -3278,14 +3283,14 @@ class TestCompositorIBusBridging(unittest.TestCase): patch.object(TextInjector, "_is_ydotoold_running", return_value=True), ): injector = TextInjector() - self.assertEqual(injector.wayland_tool, "ydotool") + self.assertEqual(injector.wayland_tool, "portal") @patch("vocalinux.text_injection.text_injector.is_ibus_available", return_value=False) @patch("vocalinux.text_injection.text_injector.shutil.which") def test_non_kde_auto_still_prefers_portal_over_ydotool( self, mock_which: MagicMock, _mock_ibus: MagicMock ) -> None: - """The ydotool-first reorder is KDE-only; other desktops keep the portal.""" + """The portal leads on every desktop that implements it, KDE included (#31).""" mock_which.side_effect = lambda cmd: { "ydotool": "/usr/bin/ydotool", "ydotoold": "/usr/bin/ydotoold", @@ -3314,7 +3319,7 @@ class TestCompositorIBusBridging(unittest.TestCase): def test_kde_auto_uses_portal_when_ydotool_missing( self, mock_which: MagicMock, _mock_ibus: MagicMock ) -> None: - """The KDE reorder needs ydotool installed; without it the portal still wins.""" + """On KDE without any typing tool installed, the portal still wins (#31).""" mock_which.side_effect = lambda cmd: None with ( patch.dict( @@ -3339,7 +3344,7 @@ class TestCompositorIBusBridging(unittest.TestCase): def test_kde_auto_uses_portal_when_ydotoold_unusable( self, mock_which: MagicMock, _mock_ibus: MagicMock ) -> None: - """On KDE the portal is still the fallback when ydotool cannot run (#911).""" + """On KDE with an unusable ydotool daemon, the portal leads (#31).""" mock_which.side_effect = lambda cmd: { "ydotool": "/usr/bin/ydotool", "ydotoold": "/usr/bin/ydotoold", diff --git a/tests/test_text_injector_ext.py b/tests/test_text_injector_ext.py index edc218a..d9bf18f 100644 --- a/tests/test_text_injector_ext.py +++ b/tests/test_text_injector_ext.py @@ -462,7 +462,7 @@ class TestCheckDependencies(unittest.TestCase): obj = _make_injector(DesktopEnvironment.WAYLAND) obj._backend_pin = ("auto", None) - config = _json.dumps({"text_injection": {"backend": "wtype"}}) + config = _json.dumps({"text_injection": {"backend": "ydotool"}}) with ( patch.dict( @@ -481,9 +481,10 @@ class TestCheckDependencies(unittest.TestCase): obj._check_dependencies() obj._warn_if_pin_not_honoured(*obj._backend_pin) - # Autodetection prefers ydotool when its daemon is ready; the saved wtype - # pin would have won had the explicit auto not discarded it. - self.assertEqual(obj.wayland_tool, "ydotool") + # Autodetection follows the decided ladder (direct typing before the + # virtual-input tool, spec #16, ticket #31); the saved ydotool pin + # would have won had the explicit auto not discarded it. + self.assertEqual(obj.wayland_tool, "wtype") self.assertFalse( [line for line in logs.output if "backend pin requested" in line], f"an explicit auto still reported a pin: {logs.output}",