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); });