173 lines
6.1 KiB
Diff
173 lines
6.1 KiB
Diff
From 7b1dc9a4bb039ff5ff2a5b71a2c6bc687ad1db72 Mon Sep 17 00:00:00 2001
|
|
From: Harald Sitter <sitter@kde.org>
|
|
Date: Thu, 2 Feb 2017 19:57:23 +0100
|
|
Subject: [PATCH] make services disqualification much stricter
|
|
|
|
Summary:
|
|
after the recent set of changes to disqualification we ended up with a bit
|
|
too lax disqualification. if we saw the exec OR the storageid we'd
|
|
hence forth disqualify a service with either of them being equal.
|
|
|
|
in case of system settings this causes a match problem. systemsettings
|
|
has two desktop files one !KDE and one OnlyKDE they are however exactly
|
|
the same except for a different name and storageid. as a result if the !KDE
|
|
one is encountered first it will be disqualified on account of
|
|
noDisplay=true and marked as seen. when the OnlyKDE systemsettings is then
|
|
iterated it will already have the Exec entry in the seen list and be
|
|
disqualified on account of that.
|
|
|
|
to prevent this type of confusion make it a requirement to match both
|
|
storageid AND exec before disqualifying.
|
|
|
|
it may actually be suitable to drop the exec altogether. say I copy firefox
|
|
into the user local applications dir and set it NoDisplay=true but change
|
|
the exec. this would still lead to the system-wide firefox being found as
|
|
it is not getting disqualified. long story short: maybe we should
|
|
disqualify services solely on the storageid?
|
|
|
|
Test Plan:
|
|
- new autotest case
|
|
- fails without changes
|
|
- passes with changes
|
|
|
|
Reviewers: broulik
|
|
|
|
Reviewed By: broulik
|
|
|
|
Subscribers: plasma-devel
|
|
|
|
Tags: #plasma
|
|
|
|
Differential Revision: https://phabricator.kde.org/D4415
|
|
---
|
|
.../autotests/fixtures/kdesystemsettings.desktop | 13 +++++++++
|
|
.../autotests/fixtures/systemsettings.desktop | 14 +++++++++
|
|
runners/services/autotests/servicerunnertest.cpp | 34 ++++++++++++++++++++++
|
|
runners/services/servicerunner.cpp | 3 +-
|
|
4 files changed, 63 insertions(+), 1 deletion(-)
|
|
create mode 100644 runners/services/autotests/fixtures/kdesystemsettings.desktop
|
|
create mode 100644 runners/services/autotests/fixtures/systemsettings.desktop
|
|
|
|
diff --git a/runners/services/autotests/fixtures/kdesystemsettings.desktop b/runners/services/autotests/fixtures/kdesystemsettings.desktop
|
|
new file mode 100644
|
|
index 00000000..0623f57a
|
|
--- /dev/null
|
|
+++ b/runners/services/autotests/fixtures/kdesystemsettings.desktop
|
|
@@ -0,0 +1,13 @@
|
|
+[Desktop Entry]
|
|
+Exec=systemsettings5
|
|
+Icon=preferences-system
|
|
+Type=Application
|
|
+X-KDE-StartupNotify=true
|
|
+NotShowIn=KDE;
|
|
+
|
|
+GenericName=KDE System Settings ServiceRunnerTest
|
|
+
|
|
+Name=KDE System Settings ServiceRunnerTest
|
|
+
|
|
+X-DBUS-StartupType=Unique
|
|
+Categories=Qt;KDE;Settings;
|
|
diff --git a/runners/services/autotests/fixtures/systemsettings.desktop b/runners/services/autotests/fixtures/systemsettings.desktop
|
|
new file mode 100644
|
|
index 00000000..935db902
|
|
--- /dev/null
|
|
+++ b/runners/services/autotests/fixtures/systemsettings.desktop
|
|
@@ -0,0 +1,14 @@
|
|
+[Desktop Entry]
|
|
+Exec=systemsettings5
|
|
+Icon=preferences-system
|
|
+Type=Application
|
|
+X-DocPath=systemsettings/index.html
|
|
+X-KDE-StartupNotify=true
|
|
+OnlyShowIn=KDE;
|
|
+
|
|
+GenericName=System Settings ServiceRunnerTest
|
|
+
|
|
+Name=System Settings ServiceRunnerTest
|
|
+
|
|
+X-DBUS-StartupType=Unique
|
|
+Categories=Qt;KDE;Settings;
|
|
diff --git a/runners/services/autotests/servicerunnertest.cpp b/runners/services/autotests/servicerunnertest.cpp
|
|
index d0c6daed..d3217768 100644
|
|
--- a/runners/services/autotests/servicerunnertest.cpp
|
|
+++ b/runners/services/autotests/servicerunnertest.cpp
|
|
@@ -38,6 +38,7 @@ private Q_SLOTS:
|
|
|
|
void testChromeAppsRelevance();
|
|
void testKonsoleVsYakuakeComment();
|
|
+ void testSystemSettings();
|
|
};
|
|
|
|
void ServiceRunnerTest::initTestCase()
|
|
@@ -58,6 +59,11 @@ void ServiceRunnerTest::initTestCase()
|
|
setlocale(LC_ALL, "C.utf8");
|
|
|
|
KSycoca::self()->ensureCacheValid();
|
|
+
|
|
+ // Make sure noDisplay behaves consistently WRT OnlyShowIn etc.
|
|
+ QVERIFY(setenv("XDG_CURRENT_DESKTOP", "KDE", 1) == 0);
|
|
+ // NOTE: noDisplay also includes X-KDE-OnlyShowOnQtPlatforms which is a bit harder to fake
|
|
+ // and not currently under testing anyway.
|
|
}
|
|
|
|
void ServiceRunnerTest::cleanupTestCase()
|
|
@@ -123,6 +129,34 @@ void ServiceRunnerTest::testKonsoleVsYakuakeComment()
|
|
QVERIFY(yakuakeFound);
|
|
}
|
|
|
|
+void ServiceRunnerTest::testSystemSettings()
|
|
+{
|
|
+ // In 5.9.0 'System Settings' suddenly didn't come back as a match for 'settings' anymore.
|
|
+ // Sytem Settings has a noKDE version and a KDE version, if the noKDE version is encountered
|
|
+ // first it will be added to the seen cache, however disqualification of already seen items
|
|
+ // may then also disqualify the KDE version of system settings on account of having already
|
|
+ // seen it. This test makes sure we find the right version.
|
|
+ ServiceRunner runner(this, QVariantList());
|
|
+ Plasma::RunnerContext context;
|
|
+ context.setQuery("settings");
|
|
+
|
|
+ runner.match(context);
|
|
+
|
|
+ bool systemSettingsFound = false;
|
|
+ bool foreignSystemSettingsFound = false;
|
|
+ for (auto match : context.matches()) {
|
|
+ qDebug() << "matched" << match.text();
|
|
+ if (match.text() == "System Settings ServiceRunnerTest") {
|
|
+ systemSettingsFound = true;
|
|
+ }
|
|
+ if (match.text() == "KDE System Settings ServiceRunnerTest") {
|
|
+ foreignSystemSettingsFound = true;
|
|
+ }
|
|
+ }
|
|
+ QVERIFY(systemSettingsFound);
|
|
+ QVERIFY(!foreignSystemSettingsFound);
|
|
+}
|
|
+
|
|
QTEST_MAIN(ServiceRunnerTest)
|
|
|
|
#include "servicerunnertest.moc"
|
|
diff --git a/runners/services/servicerunner.cpp b/runners/services/servicerunner.cpp
|
|
index 68756f91..2f840a9b 100644
|
|
--- a/runners/services/servicerunner.cpp
|
|
+++ b/runners/services/servicerunner.cpp
|
|
@@ -76,7 +76,7 @@ private:
|
|
|
|
bool hasSeen(const KService::Ptr &service)
|
|
{
|
|
- return m_seen.contains(service->storageId()) ||
|
|
+ return m_seen.contains(service->storageId()) &&
|
|
m_seen.contains(service->exec());
|
|
}
|
|
|
|
@@ -88,6 +88,7 @@ private:
|
|
bool disqualify(const KService::Ptr &service)
|
|
{
|
|
auto ret = hasSeen(service) || service->noDisplay();
|
|
+ qCDebug(RUNNER_SERVICES) << service->name() << "disqualified?" << ret;
|
|
seen(service);
|
|
return ret;
|
|
}
|
|
--
|
|
2.11.0
|
|
|