From 2a0e11e077727dceb4f90d8e7e4a10a83ec4cc64 Mon Sep 17 00:00:00 2001 From: Anay Garodia <76836303+AnayGarodia@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:12:32 -0700 Subject: [PATCH] fix: correct ydotool service setup guidance (#560) --- docs/DISTRO_COMPATIBILITY.md | 24 ++++++++++++- src/vocalinux/text_injection/text_injector.py | 35 +++++++++++++++++-- tests/test_text_injector.py | 31 +++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/DISTRO_COMPATIBILITY.md b/docs/DISTRO_COMPATIBILITY.md index 8fd19b7..ec5b408 100644 --- a/docs/DISTRO_COMPATIBILITY.md +++ b/docs/DISTRO_COMPATIBILITY.md @@ -361,7 +361,7 @@ Vocalinux requires text injection tools to work with X11 or Wayland: ### For Wayland - **IBus**: Recommended for KDE Plasma Wayland and generally the most reliable direct text input path - **wtype**: Recommended for wlroots-style Wayland compositors such as sway -- **ydotool**: Universal alternative that works with both X11 and Wayland, but requires `ydotoold`; Vocalinux uses clipboard paste with `wl-clipboard`, `xclip`, or `xsel` to avoid non-US layout scrambling +- **ydotool**: Universal alternative that works with both X11 and Wayland. Current distro packages use the per-user `ydotool.service`; source builds can install the system-level `ydotoold.service`. Vocalinux uses clipboard paste with `wl-clipboard`, `xclip`, or `xsel` to avoid non-US layout scrambling. - **xdotool**: May work via XWayland fallback ### Installation by Distribution @@ -373,6 +373,28 @@ sudo pacman -S xdotool wtype wl-clipboard xclip xsel # Arch sudo zypper install xdotool wtype wl-clipboard xclip xsel # openSUSE ``` +Ubuntu releases which package `ydotool` install a **user** unit named +`ydotool.service`. Enable it only for the account running Vocalinux: + +```bash +sudo apt install ydotool +sudo usermod -aG input $USER # then log out and back in +systemctl --user enable --now ydotool.service +``` + +Do **not** use `sudo systemctl --global enable ydotool.service`. Global user-unit +enablement also starts the input-injection daemon for the display-manager greeter +and other accounts. If it was previously enabled globally, repair the setup with: + +```bash +sudo systemctl --global disable ydotool.service +systemctl --user enable --now ydotool.service +``` + +The command is different for the Debian source-build path documented above: the +upstream install supplies the system unit `ydotoold.service`, so +`sudo systemctl enable --now ydotoold.service` is correct only for that route. + ## Manual Installation For unsupported or experimental distributions: diff --git a/src/vocalinux/text_injection/text_injector.py b/src/vocalinux/text_injection/text_injector.py index 4cef667..bdd8071 100644 --- a/src/vocalinux/text_injection/text_injector.py +++ b/src/vocalinux/text_injection/text_injector.py @@ -26,6 +26,34 @@ from .ibus_engine import ( logger = logging.getLogger(__name__) +_GLOBAL_YDOTOOL_UNIT = "/etc/systemd/user/default.target.wants/ydotool.service" + + +def _ydotool_install_guidance() -> str: + """Return package- and source-specific ydotool service instructions.""" + return ( + "Ubuntu package setup for ydotool:\n" + " sudo apt install ydotool\n" + " sudo usermod -aG input $USER # then log out and back in\n" + " systemctl --user enable --now ydotool.service\n" + " Do NOT use 'systemctl --global enable ydotool.service'; it also starts " + "ydotool in display-manager greeter sessions.\n" + "If ydotool was built from source and installed a system unit, use instead:\n" + " sudo systemctl enable --now ydotoold.service" + ) + + +def _warn_if_ydotool_globally_enabled() -> None: + """Warn when ydotool's user unit is enabled for every account, including greeters.""" + if os.path.lexists(_GLOBAL_YDOTOOL_UNIT): + logger.warning( + "ydotool.service is enabled globally at %s. This can start an input-injection " + "daemon in display-manager greeter sessions. Disable it with " + "'sudo systemctl --global disable ydotool.service', then enable it only for " + "this user with 'systemctl --user enable --now ydotool.service'.", + _GLOBAL_YDOTOOL_UNIT, + ) + def _is_kde_plasma_session() -> bool: """Return True when the current desktop session appears to be KDE Plasma.""" @@ -404,6 +432,9 @@ class TextInjector: ydotool_available = shutil.which("ydotool") is not None xdotool_available = shutil.which("xdotool") is not None + 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. if ydotool_available and self._ensure_ydotoold(): @@ -437,9 +468,7 @@ class TextInjector: "\n" "For KDE Plasma Wayland users: wtype is not supported. " f"{_kde_wayland_ibus_hint()}\n" - "Or install ydotool/wl-copy for fallback:\n" - " sudo apt install ydotool\n" - " sudo systemctl enable --now ydotoold\n" + f"Or install ydotool/wl-copy for fallback:\n{_ydotool_install_guidance()}\n" "Or for clipboard fallback: sudo apt install wl-copy" ) raise RuntimeError("Missing required dependencies for text injection") diff --git a/tests/test_text_injector.py b/tests/test_text_injector.py index 888df5a..c5a5567 100644 --- a/tests/test_text_injector.py +++ b/tests/test_text_injector.py @@ -10,7 +10,12 @@ import unittest from unittest.mock import MagicMock, patch # Update import path to use the new package structure -from vocalinux.text_injection.text_injector import DesktopEnvironment, TextInjector +from vocalinux.text_injection.text_injector import ( + DesktopEnvironment, + TextInjector, + _warn_if_ydotool_globally_enabled, + _ydotool_install_guidance, +) # Create a mock for audio feedback module mock_audio_feedback = MagicMock() @@ -936,6 +941,30 @@ class TestTextInjectorEdgeCases(unittest.TestCase): with self.assertRaises(RuntimeError): TextInjector() + def test_ydotool_package_guidance_uses_user_service(self): + """Package and source routes must clearly use their distinct unit scopes.""" + guidance = _ydotool_install_guidance() + + self.assertIn("sudo apt install ydotool", guidance) + self.assertIn("systemctl --user enable --now ydotool.service", guidance) + self.assertIn("sudo usermod -aG input $USER", guidance) + self.assertIn("sudo systemctl enable --now ydotoold.service", guidance) + self.assertIn("Do NOT use 'systemctl --global enable ydotool.service'", guidance) + + @patch("vocalinux.text_injection.text_injector.logger.warning") + @patch("vocalinux.text_injection.text_injector.os.path.lexists", return_value=True) + def test_globally_enabled_ydotool_warns_with_remediation(self, mock_lexists, mock_warning): + """A persistent global unit should produce an actionable greeter warning.""" + _warn_if_ydotool_globally_enabled() + + mock_lexists.assert_called_once_with( + "/etc/systemd/user/default.target.wants/ydotool.service" + ) + warning = mock_warning.call_args.args[0] + self.assertIn("display-manager greeter sessions", warning) + self.assertIn("systemctl --global disable ydotool.service", warning) + self.assertIn("systemctl --user enable --now ydotool.service", warning) + def test_detect_environment_unknown(self): """Test environment detection when session type is unknown and no display vars set.""" # Use clear=True to start fresh, then only set XDG_SESSION_TYPE to empty