Skip to content

Commit 3888b01

Browse files
committed
Fix hang caused by no Notifications service registered in DBus
Issue #4353 Add a check when looking up the `org.freedesktop.Notifications` service to see if the it actually exists before trying to fetch it Add a check on QDBus service lookup in screengrabber freedesktop
1 parent 88c951e commit 3888b01

File tree

3 files changed

+66
-31
lines changed

3 files changed

+66
-31
lines changed

data/translations/Internationalization_en.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,14 @@
502502
<translation type="unfinished"></translation>
503503
</message>
504504
</context>
505+
<context>
506+
<name>SystemNotification</name>
507+
<message>
508+
<location filename="../../src/utils/systemnotification.cpp" line="49"/>
509+
<source>No DBus System Notification service found</source>
510+
<translation type="unfinished"></translation>
511+
</message>
512+
</context>
505513
<context>
506514
<name>ConfigResolver</name>
507515
<message>
@@ -1779,27 +1787,32 @@ You may need to escape the &apos;#&apos; sign as in &apos;\#FFF&apos;</source>
17791787
<translation type="unfinished"></translation>
17801788
</message>
17811789
<message>
1782-
<location filename="../../src/utils/screengrabber.cpp" line="171"/>
1783-
<source>If the useGrimAdapter setting is not enabled, the dbus protocol will be used. It should be noted that using the dbus protocol under wayland is not recommended. It is recommended to enable the useGrimAdapter setting in flameshot.ini to activate the grim-based general wayland screenshot adapter</source>
1790+
<location filename="../../src/utils/screengrabber.cpp" line="70"/>
1791+
<source>Could not locate the `org.freedesktop.portal.Desktop` service</source>
17841792
<translation type="unfinished"></translation>
17851793
</message>
17861794
<message>
17871795
<location filename="../../src/utils/screengrabber.cpp" line="182"/>
1796+
<source>If the useGrimAdapter setting is not enabled, the dbus protocol will be used. It should be noted that using the dbus protocol under wayland is not recommended. It is recommended to enable the useGrimAdapter setting in flameshot.ini to activate the grim-based general wayland screenshot adapter</source>
1797+
<translation type="unfinished"></translation>
1798+
</message>
1799+
<message>
1800+
<location filename="../../src/utils/screengrabber.cpp" line="193"/>
17881801
<source>grim&apos;s screenshot component is implemented based on wlroots, it may not be used in GNOME or similar desktop environments</source>
17891802
<translation type="unfinished"></translation>
17901803
</message>
17911804
<message>
1792-
<location filename="../../src/utils/screengrabber.cpp" line="194"/>
1805+
<location filename="../../src/utils/screengrabber.cpp" line="204"/>
17931806
<source>Unable to detect desktop environment (GNOME? KDE? Qile? Sway? ...)</source>
17941807
<translation type="unfinished"></translation>
17951808
</message>
17961809
<message>
1797-
<location filename="../../src/utils/screengrabber.cpp" line="197"/>
1810+
<location filename="../../src/utils/screengrabber.cpp" line="207"/>
17981811
<source>Hint: try setting the XDG_CURRENT_DESKTOP environment variable.</source>
17991812
<translation type="unfinished"></translation>
18001813
</message>
18011814
<message>
1802-
<location filename="../../src/utils/screengrabber.cpp" line="202"/>
1815+
<location filename="../../src/utils/screengrabber.cpp" line="212"/>
18031816
<source>Unable to capture screen</source>
18041817
<translation type="unfinished"></translation>
18051818
</message>

src/utils/screengrabber.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,18 @@ void ScreenGrabber::freeDesktopPortal(bool& ok, QPixmap& res)
6161
{
6262

6363
#if !(defined(Q_OS_MACOS) || defined(Q_OS_WIN))
64+
auto* connectionInterface = QDBusConnection::sessionBus().interface();
65+
auto service = QStringLiteral("org.freedesktop.portal.Desktop");
66+
67+
if (!connectionInterface->isServiceRegistered(service)) {
68+
ok = false;
69+
AbstractLogger::error() << tr(
70+
"Could not locate the `org.freedesktop.portal.Desktop` service");
71+
return;
72+
}
73+
6474
QDBusInterface screenshotInterface(
65-
QStringLiteral("org.freedesktop.portal.Desktop"),
75+
service,
6676
QStringLiteral("/org/freedesktop/portal/desktop"),
6777
QStringLiteral("org.freedesktop.portal.Screenshot"));
6878

@@ -72,7 +82,7 @@ void ScreenGrabber::freeDesktopPortal(bool& ok, QPixmap& res)
7282

7383
// premake interface
7484
auto* request = new OrgFreedesktopPortalRequestInterface(
75-
QStringLiteral("org.freedesktop.portal.Desktop"),
85+
service,
7686
"/org/freedesktop/portal/desktop/request/" +
7787
QDBusConnection::sessionBus().baseService().remove(':').replace('.',
7888
'_') +

src/utils/systemnotification.cpp

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#include "systemnotification.h"
22
#include "src/core/flameshot.h"
3+
#include "src/utils/abstractlogger.h"
34
#include "src/utils/confighandler.h"
45
#include <QApplication>
56
#include <QUrl>
67

78
#if !(defined(Q_OS_MACOS) || defined(Q_OS_WIN))
89
#include <QDBusConnection>
10+
#include <QDBusConnectionInterface>
911
#include <QDBusInterface>
1012
#include <QDBusMessage>
1113
#else
@@ -27,12 +29,20 @@ SystemNotification::SystemNotification(QObject* parent)
2729
if (!ConfigHandler().showDesktopNotification()) {
2830
return;
2931
}
30-
m_interface =
31-
new QDBusInterface(QStringLiteral("org.freedesktop.Notifications"),
32-
QStringLiteral("/org/freedesktop/Notifications"),
33-
QStringLiteral("org.freedesktop.Notifications"),
34-
QDBusConnection::sessionBus(),
35-
this);
32+
auto bus = QDBusConnection::sessionBus();
33+
auto* connectionInterface = bus.interface();
34+
35+
auto service = QStringLiteral("org.freedesktop.Notifications");
36+
auto path = QStringLiteral("/org/freedesktop/Notifications");
37+
auto interface = QStringLiteral("org.freedesktop.Notifications");
38+
39+
if (connectionInterface->isServiceRegistered(service)) {
40+
m_interface = new QDBusInterface(service, path, interface, bus, this);
41+
} else {
42+
AbstractLogger::warning(AbstractLogger::Stderr |
43+
AbstractLogger::LogFile)
44+
<< tr("No DBus System Notification service found");
45+
}
3646
#endif
3747
}
3848

@@ -63,24 +73,26 @@ void SystemNotification::sendMessage(const QString& text,
6373
},
6474
Qt::QueuedConnection);
6575
#else
66-
QList<QVariant> args;
67-
QVariantMap hintsMap;
68-
if (!savePath.isEmpty()) {
69-
QUrl fullPath = QUrl::fromLocalFile(savePath);
70-
// allows the notification to be dragged and dropped
71-
hintsMap[QStringLiteral("x-kde-urls")] =
72-
QStringList({ fullPath.toString() });
73-
}
76+
if (nullptr != m_interface && m_interface->isValid()) {
77+
QList<QVariant> args;
78+
QVariantMap hintsMap;
79+
if (!savePath.isEmpty()) {
80+
QUrl fullPath = QUrl::fromLocalFile(savePath);
81+
// allows the notification to be dragged and dropped
82+
hintsMap[QStringLiteral("x-kde-urls")] =
83+
QStringList({ fullPath.toString() });
84+
}
7485

75-
args << (qAppName()) // appname
76-
<< static_cast<unsigned int>(0) // id
77-
<< FLAMESHOT_ICON // icon
78-
<< title // summary
79-
<< text // body
80-
<< QStringList() // actions
81-
<< hintsMap // hints
82-
<< timeout; // timeout
83-
m_interface->callWithArgumentList(
84-
QDBus::AutoDetect, QStringLiteral("Notify"), args);
86+
args << (qAppName()) // appname
87+
<< static_cast<unsigned int>(0) // id
88+
<< FLAMESHOT_ICON // icon
89+
<< title // summary
90+
<< text // body
91+
<< QStringList() // actions
92+
<< hintsMap // hints
93+
<< timeout; // timeout
94+
m_interface->callWithArgumentList(
95+
QDBus::AutoDetect, QStringLiteral("Notify"), args);
96+
}
8597
#endif
8698
}

0 commit comments

Comments
 (0)