Skip to content

Commit

Permalink
Mark progress equiped notifications as temporarily non-removable
Browse files Browse the repository at this point in the history
Progress update should arrive relatively often. Removing one
notification that would appear a second later wouldn't make sense.

Android notifications should also end up matching behavior of
ongoing notifications, though it could be considered using the
explicit property for that later on.
  • Loading branch information
pvuorela committed Jan 8, 2018
1 parent c411b12 commit 06e8ac9
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 7 deletions.
42 changes: 38 additions & 4 deletions src/notifications/lipsticknotification.cpp
Expand Up @@ -63,7 +63,8 @@ LipstickNotification::LipstickNotification(const QString &appName, const QString
m_hints(hints),
m_expireTimeout(expireTimeout),
m_priority(hints.value(LipstickNotification::HINT_PRIORITY).toInt()),
m_timestamp(hints.value(LipstickNotification::HINT_TIMESTAMP).toDateTime().toMSecsSinceEpoch())
m_timestamp(hints.value(LipstickNotification::HINT_TIMESTAMP).toDateTime().toMSecsSinceEpoch()),
m_activeProgressTimer(0)
{
updateHintValues();
}
Expand All @@ -73,7 +74,8 @@ LipstickNotification::LipstickNotification(QObject *parent) :
m_id(0),
m_expireTimeout(-1),
m_priority(0),
m_timestamp(0)
m_timestamp(0),
m_activeProgressTimer(0)
{
}

Expand All @@ -90,7 +92,8 @@ LipstickNotification::LipstickNotification(const LipstickNotification &notificat
m_hintValues(notification.m_hintValues),
m_expireTimeout(notification.m_expireTimeout),
m_priority(notification.m_priority),
m_timestamp(notification.m_timestamp)
m_timestamp(notification.m_timestamp),
m_activeProgressTimer(0) // not caring for d-bus serialization
{
}

Expand Down Expand Up @@ -302,7 +305,16 @@ QString LipstickNotification::category() const

bool LipstickNotification::isUserRemovable() const
{
return m_hints.value(LipstickNotification::HINT_USER_REMOVABLE, QVariant(true)).toBool();
if (hasProgress() && m_activeProgressTimer && m_activeProgressTimer->isActive()) {
return false;
} else {
return isUserRemovableByHint();
}
}

bool LipstickNotification::isUserRemovableByHint() const
{
return (m_hints.value(LipstickNotification::HINT_USER_REMOVABLE, QVariant(true)).toBool());
}

bool LipstickNotification::hidden() const
Expand Down Expand Up @@ -402,6 +414,28 @@ quint64 LipstickNotification::internalTimestamp() const
return m_timestamp;
}

void LipstickNotification::restartProgressTimer()
{
// if this is notification with progress, have it non-removable for some time so updates don't instantly re-add it.
// if no updates come soon, it can be considered stalled and removable.
//
// TODO: if we had an explicit hint like Android notifications' setOngoing(bool) we could use that for
// temporarily marking notification non-removable, maybe using a lot longer timer.
if (hasProgress()) {
bool wasUserRemovable = isUserRemovable();
if (!m_activeProgressTimer) {
m_activeProgressTimer = new QTimer(this);
m_activeProgressTimer->setSingleShot(true);
connect(m_activeProgressTimer, &QTimer::timeout, this, &LipstickNotification::userRemovableChanged);
}
m_activeProgressTimer->start(60000); // just need some rough value here

if (!wasUserRemovable) {
emit userRemovableChanged();
}
}
}

void LipstickNotification::updateHintValues()
{
m_hintValues.clear();
Expand Down
8 changes: 8 additions & 0 deletions src/notifications/lipsticknotification.h
Expand Up @@ -20,6 +20,7 @@
#include <QStringList>
#include <QDateTime>
#include <QVariantHash>
#include <QTimer>

class QDBusArgument;

Expand Down Expand Up @@ -241,6 +242,9 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
//! Returns the user removability of the notification
bool isUserRemovable() const;

//! Returns the user removability hint state
bool isUserRemovableByHint() const;

//! Returns true if the notification has been hidden to prevent further display
bool hidden() const;

Expand All @@ -266,6 +270,9 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
//! \internal
quint64 internalTimestamp() const;

//! \internal
void restartProgressTimer();

/*!
* Creates a copy of an existing representation of a notification.
* This constructor should only be used for populating the notification
Expand Down Expand Up @@ -364,6 +371,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
// Cached values for speeding up comparisons:
int m_priority;
quint64 m_timestamp;
QTimer *m_activeProgressTimer;
};

// Order notifications by descending priority then timestamp:
Expand Down
8 changes: 5 additions & 3 deletions src/notifications/notificationmanager.cpp
Expand Up @@ -262,7 +262,7 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
applyCategoryDefinition(&notificationData);
hints_ = notificationData.hints();

if (!notificationData.isUserRemovable() && !isPrivileged()) {
if (!notificationData.isUserRemovableByHint() && !isPrivileged()) {
qWarning() << "Persistent notification from"
<< qPrintable(pidProperties.first)
<< "dropped because of insufficent permissions";
Expand All @@ -274,7 +274,7 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
: nullptr;

if (notification) {
if (!notification->isUserRemovable() && !isPrivileged()) {
if (!notification->isUserRemovableByHint() && !isPrivileged()) {
qWarning() << "An alteration to a persistent notification by"
<< qPrintable(pidProperties.first)
<< "was ignored because of insufficent permissions";
Expand All @@ -301,6 +301,8 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
m_notifications.insert(id, notification);
}

notification->restartProgressTimer();

if (androidOrigin) {
// The app icon should also be the nemo icon
const QString icon(hints_.value(LipstickNotification::HINT_ICON).toString());
Expand Down Expand Up @@ -370,7 +372,7 @@ void NotificationManager::deleteNotification(uint id)
void NotificationManager::CloseNotification(uint id, NotificationClosedReason closeReason)
{
if (LipstickNotification *notification = m_notifications.value(id)) {
if (!notification->isUserRemovable() && !isPrivileged()) {
if (!notification->isUserRemovableByHint() && !isPrivileged()) {
qWarning() << "An application was not allowed to close a notification due to insufficient permissions";
return;
}
Expand Down

0 comments on commit 06e8ac9

Please sign in to comment.