From 87046a52dac04842b7f192f3339aa3740b952e20 Mon Sep 17 00:00:00 2001 From: Mina Maher Date: Mon, 9 Nov 2026 00:14:13 -0500 Subject: [PATCH] fix(kde): resolve focused window to its .desktop ID so picker bindings activate The "add profile" picker records an app under its .desktop file ID, but KDeDesktop::focusChanged passed KWin's desktopFileName through verbatim. On Wayland that is the window's app_id, which is not always a .desktop ID: flatpak Chrome reports "google-chrome" while its desktop file is com.google.Chrome.desktop (StartupWMClass=google-chrome). A Chrome binding created from the picker therefore never matched and Chrome stayed on Default. focusChanged now trusts desktopFileName only when it names an installed .desktop file; otherwise it resolves it (then resourceClass) through StartupWMClass / reverse-DNS tail, the same lookup Hyprland and GNOME use, and falls back to KWin's name when nothing claims the window. LinuxDesktopBase gains findDesktopFile(), which checks for an exact .desktop ID before the fuzzy scan (so native Chrome's google-chrome.desktop still wins when both builds are installed) and reports "no match" as an empty string. resolveDesktopFile() keeps its fall-back-to-input contract on top of it. Tests: picker -> binding -> KWin focus round trip against fixture desktop dirs (new setDesktopDirsForTest seam), covering flatpak Chrome plus the reverse-DNS apps already bound in the wild (Okular, Evince, Xournal++, Dolphin, Telegram) and the X11 no-desktopFileName path. --- src/core/desktop/KDeDesktop.cpp | 28 ++-- src/core/desktop/LinuxDesktopBase.cpp | 49 +++++-- src/core/desktop/LinuxDesktopBase.h | 20 ++- tests/test_wmclass_resolution.cpp | 177 ++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 23 deletions(-) diff --git a/src/core/desktop/KDeDesktop.cpp b/src/core/desktop/KDeDesktop.cpp index 422de62c..a57b3268 100644 --- a/src/core/desktop/KDeDesktop.cpp +++ b/src/core/desktop/KDeDesktop.cpp @@ -174,19 +174,25 @@ void KDeDesktop::pollActiveWindow() void KDeDesktop::focusChanged(const QString &resourceClass, const QString &title, const QString &desktopFileName) { - // Resolve to canonical app ID: the .desktop file completeBaseName. - // 1. desktopFileName from KWin (most reliable, set by well-behaved apps) - // 2. Look up .desktop file by resourceClass match (handles apps like Zoom - // where resourceClass="zoom" but .desktop is "us.zoom.Zoom.desktop") - // 3. resourceClass as final fallback - QString appId; - if (!desktopFileName.isEmpty()) { - appId = desktopFileName; - } else { - appId = resolveDesktopFile(resourceClass); - } + // Resolve to canonical app ID: the .desktop file completeBaseName, which is + // what the "add profile" picker records (runningApplications()). + // KWin's desktopFileName is the Wayland app_id and is not always a .desktop + // ID: flatpak Chrome reports "google-chrome" but ships + // com.google.Chrome.desktop with StartupWMClass=google-chrome. + // 1. desktopFileName when it names an installed .desktop file + // 2. the .desktop file claiming desktopFileName / resourceClass through + // StartupWMClass or its reverse-DNS tail ("zoom" -> "us.zoom.Zoom") + // 3. KWin's own name when no installed .desktop file claims the window + const QString kwinId = desktopFileName.isEmpty() ? resourceClass : desktopFileName; + QString appId = findDesktopFile(kwinId); + if (appId.isEmpty() && kwinId != resourceClass) + appId = findDesktopFile(resourceClass); + if (appId.isEmpty()) + appId = kwinId; if (appId == m_lastWmClass) return; + if (appId != kwinId) + qCDebug(lcFocus) << "KWin window" << kwinId << "resolved to desktop file" << appId; m_lastWmClass = appId; emit activeWindowChanged(appId, title); } diff --git a/src/core/desktop/LinuxDesktopBase.cpp b/src/core/desktop/LinuxDesktopBase.cpp index d38c6699..c36a9767 100644 --- a/src/core/desktop/LinuxDesktopBase.cpp +++ b/src/core/desktop/LinuxDesktopBase.cpp @@ -8,8 +8,11 @@ namespace logitune { -const QStringList &LinuxDesktopBase::desktopDirs() +QStringList LinuxDesktopBase::desktopDirs() const { + if (!m_desktopDirsOverride.isEmpty()) + return m_desktopDirsOverride; + static const QStringList dirs = { QStringLiteral("/usr/share/applications"), QStringLiteral("/run/host/usr/share/applications"), // host apps inside Flatpak @@ -21,17 +24,35 @@ const QStringList &LinuxDesktopBase::desktopDirs() return dirs; } -QString LinuxDesktopBase::resolveDesktopFile(const QString &resourceClass) const +QString LinuxDesktopBase::findDesktopFile(const QString &key) const { + if (key.isEmpty()) + return {}; + // Cache results — scanning .desktop dirs is slow on first call - auto cached = m_resolveCache.constFind(resourceClass); + auto cached = m_resolveCache.constFind(key); if (cached != m_resolveCache.constEnd()) return cached.value(); + const QStringList dirs = desktopDirs(); + + // Match: key already is a .desktop file ID. Checked across every directory + // before any fuzzy match so that, e.g., native Chrome's google-chrome.desktop + // wins over flatpak Chrome's com.google.Chrome.desktop (whose StartupWMClass + // is also "google-chrome"). + if (!key.contains(QLatin1Char('/'))) { + for (const QString &dir : dirs) { + if (QFileInfo::exists(dir + QLatin1Char('/') + key + QStringLiteral(".desktop"))) { + m_resolveCache.insert(key, key); + return key; + } + } + } + // Search .desktop files for one whose StartupWMClass or file name component - // matches the resourceClass. Returns the .desktop completeBaseName (canonical ID). + // matches the key. Returns the .desktop completeBaseName (canonical ID). // This handles cases like resourceClass="zoom" -> "us.zoom.Zoom.desktop" - for (const QString &dir : desktopDirs()) { + for (const QString &dir : dirs) { QDir d(dir); if (!d.exists()) continue; @@ -41,8 +62,8 @@ QString LinuxDesktopBase::resolveDesktopFile(const QString &resourceClass) const // Match: last component of filename (e.g. "Zoom" from "us.zoom.Zoom") QString shortName = baseName.contains('.') ? baseName.section('.', -1) : baseName; - if (shortName.compare(resourceClass, Qt::CaseInsensitive) == 0) { - m_resolveCache.insert(resourceClass, baseName); + if (shortName.compare(key, Qt::CaseInsensitive) == 0) { + m_resolveCache.insert(key, baseName); return baseName; } @@ -50,16 +71,22 @@ QString LinuxDesktopBase::resolveDesktopFile(const QString &resourceClass) const QSettings desktop(d.filePath(file), QSettings::IniFormat); desktop.beginGroup(QStringLiteral("Desktop Entry")); QString wmClass = desktop.value(QStringLiteral("StartupWMClass")).toString(); - if (!wmClass.isEmpty() && wmClass.compare(resourceClass, Qt::CaseInsensitive) == 0) { - m_resolveCache.insert(resourceClass, baseName); + if (!wmClass.isEmpty() && wmClass.compare(key, Qt::CaseInsensitive) == 0) { + m_resolveCache.insert(key, baseName); return baseName; } } } + m_resolveCache.insert(key, QString()); + return {}; +} + +QString LinuxDesktopBase::resolveDesktopFile(const QString &resourceClass) const +{ + const QString id = findDesktopFile(resourceClass); // No .desktop match — use resourceClass as-is - m_resolveCache.insert(resourceClass, resourceClass); - return resourceClass; + return id.isEmpty() ? resourceClass : id; } QVariantList LinuxDesktopBase::runningApplications() const diff --git a/src/core/desktop/LinuxDesktopBase.h b/src/core/desktop/LinuxDesktopBase.h index 1f14edb2..482b3b4a 100644 --- a/src/core/desktop/LinuxDesktopBase.h +++ b/src/core/desktop/LinuxDesktopBase.h @@ -9,10 +9,28 @@ class LinuxDesktopBase : public IDesktopIntegration { public: using IDesktopIntegration::IDesktopIntegration; QVariantList runningApplications() const override; + + /// Test seam: scan these directories for .desktop files instead of the + /// system application directories. + void setDesktopDirsForTest(const QStringList &dirs) + { + m_desktopDirsOverride = dirs; + m_resolveCache.clear(); + } + protected: - static const QStringList &desktopDirs(); + QStringList desktopDirs() const; + /// The .desktop file ID (completeBaseName) that `key` identifies, or an + /// empty string if no installed .desktop file claims it. `key` is a + /// .desktop ID itself, a StartupWMClass, or the last component of a + /// reverse-DNS ID ("zoom" -> "us.zoom.Zoom"). + QString findDesktopFile(const QString &key) const; + /// findDesktopFile(), falling back to `resourceClass` unchanged. QString resolveDesktopFile(const QString &resourceClass) const; mutable QHash m_resolveCache; + +private: + QStringList m_desktopDirsOverride; }; } // namespace logitune diff --git a/tests/test_wmclass_resolution.cpp b/tests/test_wmclass_resolution.cpp index f19f4d17..abcc301c 100644 --- a/tests/test_wmclass_resolution.cpp +++ b/tests/test_wmclass_resolution.cpp @@ -1,5 +1,9 @@ #include +#include +#include +#include #include "ProfileEngine.h" +#include "desktop/KDeDesktop.h" #include "helpers/TestFixtures.h" using namespace logitune; @@ -93,3 +97,176 @@ TEST_F(WmClassTest, DisplayHardwareProfileSeparation) { EXPECT_EQ(m_engine.displayProfile(kSerial), "Google Chrome"); EXPECT_EQ(m_engine.hardwareProfile(kSerial), "Dolphin"); } + +// --------------------------------------------------------------------------- +// Picker → binding → KWin focus round trip. +// +// The "add profile" picker records each app under its .desktop file ID +// (LinuxDesktopBase::runningApplications). KDeDesktop::focusChanged must +// report the same ID for that app's windows, or the binding never activates. +// KWin's desktopFileName is the Wayland app_id, which is not always a .desktop +// file ID: flatpak Chrome reports "google-chrome" while its .desktop file is +// com.google.Chrome.desktop (StartupWMClass=google-chrome). +// --------------------------------------------------------------------------- + +namespace { + +struct DesktopEntry { + const char *id; // .desktop file completeBaseName + const char *name; + const char *startupWmClass; // empty → no StartupWMClass key +}; + +struct FocusCase { + const char *label; + const char *pickerTitle; // picker entry the user chose + const char *resourceClass; // what KWin reports for the window + const char *desktopFileName; + const char *expectedAppId; // .desktop ID both sides must agree on +}; + +// Mirrors the user's machine: native apps in one dir, flatpak exports in another. +const DesktopEntry kSystemApps[] = { + {"org.gnome.Evince", "Document Viewer", ""}, + {"com.github.xournalpp.xournalpp", "Xournal++", "com.github.xournalpp.xournalpp"}, + {"org.kde.dolphin", "Dolphin", ""}, +}; +const DesktopEntry kFlatpakApps[] = { + {"com.google.Chrome", "Google Chrome", "google-chrome"}, + {"org.kde.okular", "Okular", "okular"}, + {"org.telegram.desktop", "Telegram", "TelegramDesktop"}, +}; + +void writeDesktopFile(const QString &dir, const DesktopEntry &e) +{ + QDir().mkpath(dir); + QFile f(dir + QLatin1Char('/') + QString::fromLatin1(e.id) + QStringLiteral(".desktop")); + ASSERT_TRUE(f.open(QIODevice::WriteOnly | QIODevice::Truncate)); + QByteArray body = "[Desktop Entry]\nType=Application\nName="; + body += e.name; + body += "\nExec=true\n"; + if (*e.startupWmClass) { + body += "StartupWMClass="; + body += e.startupWmClass; + body += '\n'; + } + f.write(body); +} + +void PrintTo(const FocusCase &c, std::ostream *os) { *os << c.label; } + +} // namespace + +class KdeFocusWmClassTest : public ProfileFixture { +protected: + void SetUp() override { + ProfileFixture::SetUp(); + m_systemDir = tmpPath() + QStringLiteral("/system-apps"); + m_flatpakDir = tmpPath() + QStringLiteral("/flatpak-apps"); + for (const auto &e : kSystemApps) writeDesktopFile(m_systemDir, e); + for (const auto &e : kFlatpakApps) writeDesktopFile(m_flatpakDir, e); + + const QString profilesDir = tmpPath() + QStringLiteral("/profiles"); + QDir().mkpath(profilesDir); + ProfileEngine::saveProfile(profilesDir + "/default.conf", makeDefaultProfile()); + m_engine.registerDevice(kSerial, profilesDir); + } + + QStringList dirs() const { return {m_systemDir, m_flatpakDir}; } + + // wmClass the picker stores for the entry titled `title`. + QString pickerWmClass(const QString &title) const { + KDeDesktop kde; + kde.setDesktopDirsForTest(dirs()); + for (const QVariant &v : kde.runningApplications()) { + const QVariantMap m = v.toMap(); + if (m.value(QStringLiteral("title")).toString() == title) + return m.value(QStringLiteral("wmClass")).toString(); + } + return {}; + } + + // App ID KDeDesktop emits when KWin reports a focused window. + QString focusedAppId(const QString &resourceClass, const QString &desktopFileName, + const QStringList &desktopDirs) const { + KDeDesktop kde; + kde.setDesktopDirsForTest(desktopDirs); + QSignalSpy spy(&kde, &IDesktopIntegration::activeWindowChanged); + kde.focusChanged(resourceClass, QStringLiteral("window title"), desktopFileName); + return spy.isEmpty() ? QString() : spy.takeFirst().at(0).toString(); + } + + ProfileEngine m_engine; + QString m_systemDir; + QString m_flatpakDir; + const QString kSerial = QStringLiteral("test-serial"); +}; + +// Regression: flatpak Chrome picked from the "add profile" list must activate. +// KWin reports resourceClass = desktopFileName = "google-chrome" (the Wayland +// app_id); the picker stored "com.google.Chrome". +TEST_F(KdeFocusWmClassTest, FlatpakChromePickerBindingActivatesOnFocus) { + const QString picked = pickerWmClass(QStringLiteral("Google Chrome")); + ASSERT_EQ(picked.toStdString(), "com.google.Chrome"); + m_engine.createProfileForApp(kSerial, picked, QStringLiteral("Google Chrome")); + + const QString focused = focusedAppId(QStringLiteral("google-chrome"), + QStringLiteral("google-chrome"), dirs()); + EXPECT_EQ(focused.toStdString(), picked.toStdString()); + EXPECT_EQ(m_engine.profileForApp(kSerial, focused).toStdString(), "Google Chrome"); +} + +// A real .desktop ID from KWin wins over a StartupWMClass match in another +// file: native Chrome ships google-chrome.desktop, so bindings made for it +// keep working even when flatpak Chrome (StartupWMClass=google-chrome) is +// installed too and its directory is scanned first. +TEST_F(KdeFocusWmClassTest, ExistingDesktopFileNameIsTrustedVerbatim) { + writeDesktopFile(m_systemDir, {"google-chrome", "Google Chrome (native)", "Google-chrome"}); + const QString focused = focusedAppId(QStringLiteral("google-chrome"), + QStringLiteral("google-chrome"), + {m_flatpakDir, m_systemDir}); + EXPECT_EQ(focused.toStdString(), "google-chrome"); +} + +// Apps with no .desktop file at all keep KWin's desktopFileName. +TEST_F(KdeFocusWmClassTest, UnknownAppFallsBackToDesktopFileName) { + EXPECT_EQ(focusedAppId(QStringLiteral("some-tool"), QStringLiteral("some-tool"), dirs()) + .toStdString(), "some-tool"); + EXPECT_EQ(focusedAppId(QStringLiteral("some-tool"), QString(), dirs()).toStdString(), + "some-tool"); +} + +class KdeFocusRoundTrip : public KdeFocusWmClassTest, + public ::testing::WithParamInterface {}; + +// Reverse-DNS apps whose KWin desktopFileName already is their .desktop ID +// (or that only report a StartupWMClass) must keep round-tripping. +TEST_P(KdeFocusRoundTrip, PickerBindingActivatesOnFocus) { + const FocusCase &c = GetParam(); + const QString picked = pickerWmClass(QString::fromLatin1(c.pickerTitle)); + ASSERT_EQ(picked.toStdString(), c.expectedAppId); + m_engine.createProfileForApp(kSerial, picked, QString::fromLatin1(c.pickerTitle)); + + const QString focused = focusedAppId(QString::fromLatin1(c.resourceClass), + QString::fromLatin1(c.desktopFileName), dirs()); + EXPECT_EQ(focused.toStdString(), picked.toStdString()); + EXPECT_EQ(m_engine.profileForApp(kSerial, focused).toStdString(), c.pickerTitle); +} + +INSTANTIATE_TEST_SUITE_P( + ReverseDnsApps, KdeFocusRoundTrip, + ::testing::Values( + // Evince and Dolphin values were read from a live KDE Plasma 6 Wayland + // session (resourceClass / desktopFileName of each window). + FocusCase{"Evince", "Document Viewer", "org.gnome.Evince", "org.gnome.Evince", "org.gnome.Evince"}, + FocusCase{"Dolphin", "Dolphin", "org.kde.dolphin", "org.kde.dolphin", "org.kde.dolphin"}, + FocusCase{"FlatpakOkular", "Okular", "org.kde.okular", "org.kde.okular", "org.kde.okular"}, + FocusCase{"Xournalpp", "Xournal++", "com.github.xournalpp.xournalpp", + "com.github.xournalpp.xournalpp", + "com.github.xournalpp.xournalpp"}, + FocusCase{"FlatpakTelegram", "Telegram", "org.telegram.desktop", "org.telegram.desktop", + "org.telegram.desktop"}, + // X11 windows can arrive with no desktopFileName; StartupWMClass maps them. + FocusCase{"OkularNoDesktopFileName", "Okular", "okular", "", "org.kde.okular"}, + FocusCase{"TelegramX11", "Telegram", "TelegramDesktop", "", "org.telegram.desktop"}), + [](const ::testing::TestParamInfo &info) { return std::string(info.param.label); });