Commit 7b1dc9a4 authored by Harald Sitter's avatar Harald Sitter
Browse files

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
parent 08dff33f
[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;
[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;
......@@ -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"
......@@ -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;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment