From 93128360371954698f21fc9f6397a513ac269f5f Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Fri, 11 Oct 2024 10:43:23 -0400 Subject: [PATCH] DesktopFileManager::resolve_app_id no longer returns an app id with a .desktop file suffix --- .../frontend_wayland/desktop_file_manager.cpp | 13 +++++-------- .../foreign_toplevel_manager_v1.cpp | 17 +++++++++++------ .../test_desktop_file_manager.cpp | 12 ++++++------ .../test_g_desktop_file_cache.cpp | 8 ++++---- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/server/frontend_wayland/desktop_file_manager.cpp b/src/server/frontend_wayland/desktop_file_manager.cpp index 013fb40d2bb..c8f4abefaa2 100644 --- a/src/server/frontend_wayland/desktop_file_manager.cpp +++ b/src/server/frontend_wayland/desktop_file_manager.cpp @@ -31,10 +31,8 @@ namespace mf = mir::frontend; namespace { -const char* DESKTOP_FILE_POSTFIX = ".desktop"; - std::pair const app_id_to_desktop_file_quirks[] = { - {"gnome-terminal-server", "org.gnome.Terminal.desktop"} // https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/8033 + {"gnome-terminal-server", "org.gnome.Terminal"} // https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/8033 }; inline void rtrim(std::string &s) { @@ -88,8 +86,7 @@ std::string mf::DesktopFileManager::resolve_app_id(const scene::Surface* surface } // Second, let's see if we can map it straight to a desktop file - auto desktop_file = app_id + DESKTOP_FILE_POSTFIX; - auto found = lookup_basename(desktop_file); + auto found = lookup_basename(app_id); if (found) { mir::log_info("Successfully resolved app id from basename, id=%s", found->id.c_str()); @@ -97,7 +94,7 @@ std::string mf::DesktopFileManager::resolve_app_id(const scene::Surface* surface } // Third, lowercase it and try again - auto lowercase_desktop_file = desktop_file; + auto lowercase_desktop_file = app_id; for(char &ch : lowercase_desktop_file) ch = std::tolower(ch); @@ -186,7 +183,7 @@ std::string mf::DesktopFileManager::parse_snap_security_profile_to_desktop_id(st if (c == '.') c = '_'; - return sandboxed_app_id + DESKTOP_FILE_POSTFIX; + return sandboxed_app_id; } std::shared_ptr mf::DesktopFileManager::resolve_if_snap(int pid, mir::Fd const& socket_fd) @@ -265,7 +262,7 @@ std::shared_ptr mf::DesktopFileManager::resolve_if_flatpak(int if (!sandboxed_app_id) return nullptr; - return cache->lookup_by_app_id(std::string(sandboxed_app_id) + DESKTOP_FILE_POSTFIX); + return cache->lookup_by_app_id(std::string(sandboxed_app_id)); } std::shared_ptr mf::DesktopFileManager::resolve_if_executable_matches(int pid) diff --git a/src/server/frontend_wayland/foreign_toplevel_manager_v1.cpp b/src/server/frontend_wayland/foreign_toplevel_manager_v1.cpp index 22c502c6af8..64072100a84 100644 --- a/src/server/frontend_wayland/foreign_toplevel_manager_v1.cpp +++ b/src/server/frontend_wayland/foreign_toplevel_manager_v1.cpp @@ -561,7 +561,7 @@ void mf::GDesktopFileCache::refresh_app_cache() { GAppInfo *app_info = static_cast(info->data); - const char* id = g_app_info_get_id(app_info); + std::string id = g_app_info_get_id(app_info); const char* wm_class = g_desktop_app_info_get_startup_wm_class(G_DESKTOP_APP_INFO(app_info)); const char* exec = g_app_info_get_executable(app_info); @@ -577,19 +577,24 @@ void mf::GDesktopFileCache::refresh_app_cache() continue; } - std::shared_ptr file = std::make_shared(id, wm_class, exec); - const char* app_id = g_app_info_get_id(app_info); + // It is likely that [id] ends in a .desktop suffix. If that's the case, we will strip + // it out since that isn't useful in this context. + const char* const DESKTOP_PREFIX = ".desktop"; + if (id.ends_with(DESKTOP_PREFIX)) + id.erase(id.length() - strlen(DESKTOP_PREFIX)); + + std::shared_ptr file = std::make_shared(id.c_str(), wm_class, exec); if (g_app_info_should_show(app_info)) { - new_id_to_app[app_id] = file; + new_id_to_app[id] = file; new_exec_to_app[exec] = file; } else { - new_hidden_id_to_app[app_id] = file; + new_hidden_id_to_app[id] = file; } - if (wm_class == NULL) + if (!wm_class) continue; new_wm_class_to_app_info[wm_class] = file; diff --git a/tests/unit-tests/frontend_wayland/test_desktop_file_manager.cpp b/tests/unit-tests/frontend_wayland/test_desktop_file_manager.cpp index 8ed4174a2c8..9e7eefb330e 100644 --- a/tests/unit-tests/frontend_wayland/test_desktop_file_manager.cpp +++ b/tests/unit-tests/frontend_wayland/test_desktop_file_manager.cpp @@ -79,7 +79,7 @@ struct DesktopFileManager : Test std::shared_ptr session; const char* APPLICATION_ID = "test_app_id"; - const char* DESKTOP_FILE_APP_ID = "test_app_id.desktop"; + const char* DESKTOP_FILE_APP_ID = "test_app_id"; const int PID = -12345; // Negative so it never accidentally works void SetUp() override @@ -116,7 +116,7 @@ TEST_F(DesktopFileManager, can_find_gnome_terminal_server) ON_CALL(surface, application_id()) .WillByDefault(testing::Invoke([]() { return "gnome-terminal-server"; })); auto app_id = file_manager->resolve_app_id(&surface); - EXPECT_THAT(app_id, "org.gnome.Terminal.desktop"); + EXPECT_THAT(app_id, "org.gnome.Terminal"); } TEST_F(DesktopFileManager, can_find_app_when_wm_class_matches) @@ -177,19 +177,19 @@ TEST_F(DesktopFileManager, when_security_profile_does_not_start_with_prefix_then TEST_F(DesktopFileManager, when_security_profile_is_valid_then_desktop_id_is_returned) { auto result = mf::DesktopFileManager::parse_snap_security_profile_to_desktop_id("snap.firefox.firefox (enforce)"); - EXPECT_EQ(result, "firefox_firefox.desktop"); + EXPECT_EQ(result, "firefox_firefox"); } TEST_F(DesktopFileManager, when_security_profile_is_valid_and_lacks_protection_indication_then_desktop_id_is_returned) { auto result = mf::DesktopFileManager::parse_snap_security_profile_to_desktop_id("snap.firefox.firefox"); - EXPECT_EQ(result, "firefox_firefox.desktop"); + EXPECT_EQ(result, "firefox_firefox"); } TEST_F(DesktopFileManager, can_resolve_from_valid_flatpak_info) { const char* app_id = "test.application.name"; - const char* desktop_app_id = "test.application.name.desktop"; + const char* desktop_app_id = "test.application.name"; std::stringstream flatpak_info_path; flatpak_info_path << "/proc/" << PID << "/root/.flatpak-info"; @@ -223,7 +223,7 @@ TEST_F(DesktopFileManager, can_resolve_from_valid_flatpak_info) TEST_F(DesktopFileManager, app_id_will_not_resolve_from_flatpak_info_when_name_is_missing) { - const char* desktop_app_id = "test.application.name.desktop"; + const char* desktop_app_id = "test.application.name"; std::stringstream flatpak_info_path; flatpak_info_path << "/proc/" << PID << "/root/.flatpak-info"; diff --git a/tests/unit-tests/frontend_wayland/test_g_desktop_file_cache.cpp b/tests/unit-tests/frontend_wayland/test_g_desktop_file_cache.cpp index 8a426a9806e..b6b475e15cd 100644 --- a/tests/unit-tests/frontend_wayland/test_g_desktop_file_cache.cpp +++ b/tests/unit-tests/frontend_wayland/test_g_desktop_file_cache.cpp @@ -75,7 +75,7 @@ TEST_F(GDesktopFileCache, when_desktop_file_is_in_directory_then_it_can_be_found write_desktop_file(sh_desktop_contents, "mir.test.info.desktop"); mf::GDesktopFileCache cache(main_loop); - auto file = cache.lookup_by_app_id("mir.test.info.desktop"); + auto file = cache.lookup_by_app_id("mir.test.info"); ASSERT_THAT(file, NotNull()); } @@ -101,7 +101,7 @@ TEST_F(GDesktopFileCache, when_desktop_file_lacks_exec_then_it_cannot_be_found) write_desktop_file(sh_desktop_contents, "mir.test.info.desktop"); mf::GDesktopFileCache cache(main_loop); - auto file = cache.lookup_by_app_id("mir.test.info.desktop"); + auto file = cache.lookup_by_app_id("mir.test.info"); ASSERT_THAT(file, IsNull()); } @@ -115,7 +115,7 @@ TEST_F(GDesktopFileCache, when_desktop_file_is_set_to_nodisplay_then_it_can_stil write_desktop_file(sh_desktop_contents, "mir.test.info.desktop"); mf::GDesktopFileCache cache(main_loop); - auto file = cache.lookup_by_app_id("mir.test.info.desktop"); + auto file = cache.lookup_by_app_id("mir.test.info"); ASSERT_THAT(file, NotNull()); } @@ -129,6 +129,6 @@ TEST_F(GDesktopFileCache, when_a_desktop_has_invald_exec_then_it_cannot_be_found write_desktop_file(sh_desktop_contents, "mir.test.info.desktop"); mf::GDesktopFileCache cache(main_loop); - auto file = cache.lookup_by_app_id("mir.test.info.desktop"); + auto file = cache.lookup_by_app_id("mir.test.info"); ASSERT_THAT(file, IsNull()); }