fix(screengrab): don't try grim on non-wlroots

CHANGELOG

@@ -2,14 +2,16 @@
 
 # Changelog
 
-## v0.5.5 (upcoming)
+## v0.5.5 (2024-04-30)
 
 - All: Add russian translation. Thanks, [@ViktorOn](https://github.com/ViktorOn)! ([#611](https://github.com/dynobo/normcap/pull/611))
-- All: Update spanish translation. Thanks, [@haggen88](https://github.com/haggen88)! ([#614](https://github.com/dynobo/normcap/pull/614))
+- All: Add italian translation. Thanks, [@albanobattistella](https://github.com/albanobattistella)! ([#622](https://github.com/dynobo/normcap/pull/622))
+- All: Add spanish translation. Thanks, [@haggen88](https://github.com/haggen88)! ([#614](https://github.com/dynobo/normcap/pull/614))
 - Linux: Add `xsel` as another clipboard handler. Try `--clipboard-handler xsel` if
   Normcap's result isn't copied to the clipboard correctly.
 - Linux: Fix clipboard on Gnome 46 + Wayland. ([#620](https://github.com/dynobo/normcap/pull/620))
 - Linux: Auto-remove screenshot file from pictures-directory on Wayland. Thanks, [@PavelDobCZ23](https://github.com/PavelDobCZ23)! ([#600](https://github.com/dynobo/normcap/issues/600))
+- Linux: Fix `grim` being tried for screenshots on non-compatible compositors.
 
 ## v0.5.4 (2024-01-15)
 

normcap/gui/system_info.py

@@ -57,6 +57,7 @@ def is_flatpak_package() -> bool:
 
 
 def is_prebuilt_package() -> bool:
+    # TODO: Fix usage of this function and rename!
     return is_briefcase_package() or is_flatpak_package()
 
 

normcap/resources/locales/README.md

@@ -19,7 +19,7 @@
 | [es_ES](./es_ES/LC_MESSAGES/messages.po) |     100% |   68 of 68 |
 | [fr_FR](./fr_FR/LC_MESSAGES/messages.po) |       8% |    6 of 68 |
 | [hi_IN](./hi_IN/LC_MESSAGES/messages.po) |       8% |    6 of 68 |
-| [it_IT](./it_IT/LC_MESSAGES/messages.po) |       8% |    6 of 68 |
+| [it_IT](./it_IT/LC_MESSAGES/messages.po) |     100% |   68 of 68 |
 | [pl_PL](./pl_PL/LC_MESSAGES/messages.po) |       8% |    6 of 68 |
 | [pt_PT](./pt_PT/LC_MESSAGES/messages.po) |       8% |    6 of 68 |
 | [ru_RU](./ru_RU/LC_MESSAGES/messages.po) |     100% |   68 of 68 |

normcap/screengrab/handlers/dbus_portal.py

@@ -214,10 +214,12 @@ def _exception_triggered(uri: Exception) -> None:
 
 
 def is_compatible() -> bool:
+    # TODO: Specify closer!
     return sys.platform == "linux"
 
 
 def is_installed() -> bool:
+    # TODO: What's with KDE?
     gnome_version = system_info.get_gnome_version()
     return not gnome_version or gnome_version >= "41"
 

normcap/screengrab/handlers/grim.py

@@ -7,6 +7,7 @@
 
 from PySide6 import QtGui
 
+from normcap.screengrab import system_info
 from normcap.screengrab.post_processing import split_full_desktop_to_screens
 
 logger = logging.getLogger(__name__)
@@ -15,7 +16,11 @@
 
 
 def is_compatible() -> bool:
-    return sys.platform == "linux"
+    return (
+        sys.platform == "linux"
+        and system_info.has_wayland_display_manager()
+        and system_info.has_wlroots_compositor()
+    )
 
 
 def is_installed() -> bool:

normcap/screengrab/handlers/qt.py

@@ -11,7 +11,7 @@
 
 
 def is_compatible() -> bool:
-    return sys.platform != "linux" or not system_info.os_has_wayland_display_manager()
+    return sys.platform != "linux" or not system_info.has_wayland_display_manager()
 
 
 def is_installed() -> bool:

normcap/screengrab/permissions.py

@@ -222,9 +222,9 @@ def has_screenshot_permission() -> bool:
     logger.debug("Checking screenshot permission")
     if sys.platform == "darwin":
         return _macos_has_screenshot_permission()
-    if sys.platform == "linux" and not system_info.os_has_wayland_display_manager():
+    if sys.platform == "linux" and not system_info.has_wayland_display_manager():
         return True
-    if sys.platform == "linux" and system_info.os_has_wayland_display_manager():
+    if sys.platform == "linux" and system_info.has_wayland_display_manager():
         return _dbus_portal_has_screenshot_permission()
     if sys.platform == "win32":
         return True
@@ -242,14 +242,14 @@ def request_screenshot_permission(
         )
         return
 
-    if sys.platform == "linux" and not system_info.os_has_wayland_display_manager():
+    if sys.platform == "linux" and not system_info.has_wayland_display_manager():
         logger.debug(
             "Not necessary to request screenshot permission on Linux, if the "
             "display manager is not Wayland. Skipping."
         )
         return
 
-    if sys.platform == "linux" and system_info.os_has_wayland_display_manager():
+    if sys.platform == "linux" and system_info.has_wayland_display_manager():
         logger.debug("Show request permission dialog.")
         dbus_portal_show_request_permission_dialog(
             title=dialog_title, text=linux_dialog_text

normcap/screengrab/system_info.py

@@ -9,7 +9,35 @@
 logger = logging.getLogger(__name__)
 
 
-def os_has_wayland_display_manager() -> bool:
+def has_wlroots_compositor() -> bool:
+    """Check if wlroots compositor is running, as grim only supports wlroots.
+
+    Certainly not wlroots based are: KDE, GNOME and Unity.
+    Others are likely wlroots based.
+    """
+    if sys.platform != "linux":
+        return False
+
+    kde_full_session = os.environ.get("KDE_FULL_SESSION", "").lower()
+    xdg_current_desktop = os.environ.get("XDG_CURRENT_DESKTOP", "").lower()
+    desktop_session = os.environ.get("DESKTOP_SESSION", "").lower()
+    gnome_desktop_session_id = os.environ.get("GNOME_DESKTOP_SESSION_ID", "")
+    if gnome_desktop_session_id == "this-is-deprecated":
+        gnome_desktop_session_id = ""
+
+    if (
+        gnome_desktop_session_id
+        or "gnome" in xdg_current_desktop
+        or kde_full_session
+        or "kde-plasma" in desktop_session
+        or "unity" in xdg_current_desktop
+    ):
+        return False
+
+    return True
+
+
+def has_wayland_display_manager() -> bool:
     """Identify relevant display managers (Linux)."""
     if sys.platform != "linux":
         return False

tests/conftest.py

@@ -34,7 +34,7 @@ def _clear_caches():
 
 @pytest.fixture()
 def temp_settings(qapp):
-    settings = QtCore.QSettings("dynobo", "normcap_tests")
+    settings = QtCore.QSettings("normcap_tests", "settings")
     yield settings
     settings.remove("")
 

tests/tests_gui/test_window.py

@@ -128,14 +128,19 @@ def test_window_get_capture_mode_fallback_to_parse(temp_settings, caplog):
     )
     invalid_mode = "some_deprecated_mode"
     temp_settings.setValue("mode", invalid_mode)
+    assert str(temp_settings.value("mode")) == invalid_mode
     win = window.Window(screen=screen, settings=temp_settings, parent=None)
+    assert str(win.settings.value("mode")) == invalid_mode
+    assert str(temp_settings.value("mode")) == invalid_mode
 
     # WHEN the capture mode is read
     mode = win.get_capture_mode()
 
     # THEN a warning should be logged
     #    and "parse" mode should be returned as fallback
+    assert temp_settings.value("mode") == invalid_mode
+    assert win.settings.value("mode") == invalid_mode
     assert "warning" in caplog.text.lower()
     assert "unknown capture mode" in caplog.text.lower()
-    assert invalid_mode in caplog.text.lower()
+    assert invalid_mode in caplog.text.lower()  # FIXME: Fails on Windows?
     assert mode == models.CaptureMode.PARSE

tests/tests_screengrab/test_handlers/test_grim.py

@@ -14,7 +14,7 @@
 )
 def test_capture_on_wayland(qapp):
     # GIVEN a linux system with grim support (Linux with compatible Wayland Compositor)
-    assert system_info.os_has_wayland_display_manager()
+    assert system_info.has_wayland_display_manager()
 
     # WHEN screenshot is taking using QT
     images = grim.capture()
@@ -62,6 +62,8 @@ def mocked_run(args, **kwargs):
     monkeypatch.setattr(grim.sys, "platform", "linux")
     monkeypatch.setattr(grim.shutil, "which", lambda *_: "/some/path")
     monkeypatch.setattr(grim.subprocess, "run", mocked_run)
+    monkeypatch.setattr(system_info, "has_wlroots_compositor", lambda: True)
+    monkeypatch.setenv("WAYLAND_DISPLAY", "wayland")
 
     assert grim.is_compatible()
     assert grim.is_installed()

tests/tests_screengrab/test_handlers/test_qt.py

@@ -7,7 +7,7 @@
 
 @pytest.mark.gui()
 @pytest.mark.skipif(
-    system_info.os_has_wayland_display_manager(), reason="Non-Wayland specific test"
+    system_info.has_wayland_display_manager(), reason="Non-Wayland specific test"
 )
 def test_capture_on_non_wayland(qapp):
     # GIVEN any operating system
@@ -22,7 +22,7 @@ def test_capture_on_non_wayland(qapp):
 
 @pytest.mark.gui()
 @pytest.mark.skipif(
-    not system_info.os_has_wayland_display_manager(), reason="Wayland specific test"
+    not system_info.has_wayland_display_manager(), reason="Wayland specific test"
 )
 def test_capture_on_wayland(qapp):
     # GIVEN any operating system

tests/tests_screengrab/test_main.py

@@ -22,40 +22,42 @@ def test_capture(qapp):
         "sys_platform",
         "gnome_version",
         "is_wayland",
+        "is_wlroots",
         "has_grim",
         "expected_handler",
     ),
     [
-        ("win32", "", True, True, Handler.QT),
-        ("win32", "", False, False, Handler.QT),
-        ("darwin", "", True, True, Handler.QT),
-        ("darwin", "", False, False, Handler.QT),
-        ("linux", "", False, True, Handler.QT),
-        ("linux", "", True, True, Handler.GRIM),
-        ("linux", "", True, False, Handler.DBUS_PORTAL),
-        ("linux", "41.0", True, False, Handler.DBUS_PORTAL),
-        ("linux", "41.0", False, False, Handler.QT),
-        ("linux", "40.0", True, False, Handler.DBUS_SHELL),
-        ("linux", "40.0", False, False, Handler.QT),
+        ("win32", "", True, True, True, Handler.QT),
+        ("win32", "", False, False, False, Handler.QT),
+        ("darwin", "", True, True, True, Handler.QT),
+        ("darwin", "", False, False, False, Handler.QT),
+        ("linux", "", False, True, True, Handler.QT),
+        ("linux", "", True, True, True, Handler.GRIM),
+        ("linux", "", True, False, True, Handler.DBUS_PORTAL),
+        ("linux", "", True, False, False, Handler.DBUS_PORTAL),
+        ("linux", "41.0", True, False, False, Handler.DBUS_PORTAL),
+        ("linux", "41.0", False, False, False, Handler.QT),
+        ("linux", "40.0", True, False, False, Handler.DBUS_SHELL),
+        ("linux", "40.0", False, False, False, Handler.QT),
     ],
 )
 def test_get_available_handlers(
     monkeypatch,
     sys_platform,
     gnome_version,
     is_wayland,
+    is_wlroots,
     has_grim,
     expected_handler,
 ):
     monkeypatch.setattr(screengrab.system_info.sys, "platform", sys_platform)
-    monkeypatch.setattr(
-        system_info, "os_has_wayland_display_manager", lambda: is_wayland
-    )
+    monkeypatch.setattr(system_info, "has_wayland_display_manager", lambda: is_wayland)
+    monkeypatch.setattr(system_info, "has_wlroots_compositor", lambda: is_wlroots)
     monkeypatch.setattr(system_info, "get_gnome_version", lambda: gnome_version)
     monkeypatch.setattr(
         grim.shutil, "which", lambda _: "/usr/bin/grim" if has_grim else None
     )
 
     handlers = screengrab.main.get_available_handlers()
 
-    assert handlers[0] == expected_handler
+    assert handlers[0] == expected_handler, handlers

tests/tests_screengrab/test_system_info.py

@@ -6,7 +6,7 @@
 # TODO: Parametrize tests?
 def test_display_manager_is_wayland_on_windows(monkeypatch):
     monkeypatch.setattr(system_info.sys, "platform", "win32")
-    is_wayland = system_info.os_has_wayland_display_manager()
+    is_wayland = system_info.has_wayland_display_manager()
     assert is_wayland is False
 
 
@@ -15,17 +15,17 @@ def test_display_manager_is_wayland_on_linux_xdg_session_type(monkeypatch):
 
     monkeypatch.setenv("XDG_SESSION_TYPE", "wayland")
     monkeypatch.setenv("WAYLAND_DISPLAY", "")
-    is_wayland = system_info.os_has_wayland_display_manager()
+    is_wayland = system_info.has_wayland_display_manager()
     assert is_wayland is True
 
     monkeypatch.setenv("XDG_SESSION_TYPE", "")
     monkeypatch.setenv("WAYLAND_DISPLAY", "wayland-0")
-    is_wayland = system_info.os_has_wayland_display_manager()
+    is_wayland = system_info.has_wayland_display_manager()
     assert is_wayland is True
 
     monkeypatch.setenv("XDG_SESSION_TYPE", "gnome-shell")
     monkeypatch.setenv("WAYLAND_DISPLAY", "")
-    is_wayland = system_info.os_has_wayland_display_manager()
+    is_wayland = system_info.has_wayland_display_manager()
     assert is_wayland is False