Skip to content

Commit

Permalink
[lipstick] Remove app_icon from hints. Contributes to JB#51351
Browse files Browse the repository at this point in the history
Reverting some parts of commit 6c5d366, the app_icon is not really
a standard hint, only the same named parameter is standard.
We don't gain much from having a duplicated extension.

Also to help the UI side, added icon source property to indicate
how reliable the value really is.
  • Loading branch information
pvuorela committed Sep 28, 2020
1 parent 016f38b commit e3d8367
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 44 deletions.
5 changes: 1 addition & 4 deletions doc/src/notifications.dox
Expand Up @@ -110,7 +110,6 @@
* - \c hints should contain the following:
* - \c category should be "im.received" to categorize the notification to be related to an instant message
* - \c urgency should be 1 (normal) since chat messages are not specifically low or high priority
* - \c app_icon should be "icon-lock-chat" to define that the icon with that ID is to be shown on the notification area
* - \c x-nemo-preview-summary should match the summary text ("John Doe") in order to show it also on the preview banner
* - \c x-nemo-preview-body should match the body text ("Hi!") in order to show it also on the preview banner
* - \c x-nemo-timestamp should be set to the time when the chat message was sent (or received, depending on the intended application logic)
Expand All @@ -133,7 +132,6 @@
* - \c hints should contain the following:
* - \c category should be "im.received" to categorize the notification to be related to an instant message
* - \c urgency should be 1 (normal) since chat messages are not specifically low or high priority
* - \c app_icon should be "icon-lock-chat" to define that the icon with that ID is to be shown on the notification area
* - \c x-nemo-item-count should be 2 to make the notification represent two chat messages
* - \c x-nemo-preview-summary should contain a brief description about the notification to be shown on the preview banner, like "John Doe"
* - \c x-nemo-preview-body should contain informative text about the notification to be shown on the preview banner, like "Are you there?"
Expand Down Expand Up @@ -165,15 +163,14 @@
* When calling Notify() to a display a transient notification, the parameters should be set as follows:
* - \c app_name should be a string identifying the sender application, such as the name of its binary, for example. "batterynotifier"
* - \c replaces_id should be 0 since the notification is a new one and not related to any existing notification
* - \c app_icon should be left empty; it will not be used in this scenario
* - \c app_icon should be "icon-battery-low" to define that the icon with that ID is to be shown on the preview banner
* - \c summary should be left empty for nothing to be shown in the events view
* - \c body should be left empty for nothing to be shown in the events view
* - \c actions should be left empty
* - \c hints should contain the following:
* - \c category should be "device" to categorize the notification to be related to the device
* - \c urgency should be 2 (critical) to show the notification over the lock screen
* - \c transient should be true to automatically close the notification after display
* - \c app_icon should be "icon-battery-low" to define that the icon with that ID is to be shown on the preview banner
* - \c x-nemo-preview-body should be "Battery low" in order to show it on the preview banner
* - \c expire_timeout should be -1 to let the notification manager choose an appropriate expiration time
*
Expand Down
44 changes: 35 additions & 9 deletions src/notifications/lipsticknotification.cpp
Expand Up @@ -35,7 +35,6 @@ const char *LipstickNotification::HINT_IMAGE_PATH = "image-path";
const char *LipstickNotification::HINT_IMAGE_DATA = "image-data";
const char *LipstickNotification::HINT_SUPPRESS_SOUND = "suppress-sound";
const char *LipstickNotification::HINT_SOUND_FILE = "sound-file";
const char *LipstickNotification::HINT_APP_ICON = "app_icon";
const char *LipstickNotification::HINT_ITEM_COUNT = "x-nemo-item-count";
const char *LipstickNotification::HINT_PRIORITY = "x-nemo-priority";
const char *LipstickNotification::HINT_TIMESTAMP = "x-nemo-timestamp";
Expand Down Expand Up @@ -65,6 +64,7 @@ LipstickNotification::LipstickNotification(const QString &appName, const QString
m_explicitAppName(explicitAppName),
m_disambiguatedAppName(disambiguatedAppName),
m_id(id),
m_appIcon(appIcon),
m_summary(summary),
m_body(body),
m_actions(actions),
Expand All @@ -74,9 +74,6 @@ LipstickNotification::LipstickNotification(const QString &appName, const QString
m_timestamp(hints.value(LipstickNotification::HINT_TIMESTAMP).toDateTime().toMSecsSinceEpoch()),
m_activeProgressTimer(0)
{
if (!appIcon.isEmpty()) {
m_hints.insert(LipstickNotification::HINT_APP_ICON, appIcon);
}
updateHintValues();
}

Expand All @@ -96,6 +93,8 @@ LipstickNotification::LipstickNotification(const LipstickNotification &notificat
m_explicitAppName(notification.m_explicitAppName),
m_disambiguatedAppName(notification.m_disambiguatedAppName),
m_id(notification.m_id),
m_appIcon(notification.m_appIcon),
m_appIconOrigin(notification.m_appIconOrigin),
m_summary(notification.m_summary),
m_body(notification.m_body),
m_actions(notification.m_actions),
Expand Down Expand Up @@ -145,7 +144,33 @@ uint LipstickNotification::id() const

QString LipstickNotification::appIcon() const
{
return m_hints.value(LipstickNotification::HINT_APP_ICON).toString();
return m_appIcon;
}

void LipstickNotification::setAppIcon(const QString &appIcon, int source)
{
bool iconChanged = false;
bool sourceChanged = false;

if (appIcon != m_appIcon) {
m_appIcon = appIcon;
}

if (source != m_appIconOrigin) {
m_appIconOrigin = source;
}

if (iconChanged) {
emit appIconChanged();
}
if (sourceChanged) {
emit appIconOriginChanged();
}
}

int LipstickNotification::appIconOrigin() const
{
return m_appIconOrigin;
}

QString LipstickNotification::summary() const
Expand Down Expand Up @@ -450,16 +475,15 @@ void LipstickNotification::updateHintValues()

if (hint == HINT_ICON) {
qWarning() << "Notification sets deprecated hint" << HINT_ICON
<< "to" << it.value() << ", use" << LipstickNotification::HINT_APP_ICON << "or"
<< "to" << it.value() << ", use app_icon parameter or"
<< LipstickNotification::HINT_IMAGE_PATH << "instead";
} else if (hint == HINT_PREVIEW_ICON) {
qWarning() << "Notification sets deprecated hint" << HINT_PREVIEW_ICON
<< "to" << it.value() << ", use" << LipstickNotification::HINT_APP_ICON << "or"
<< "to" << it.value() << ", use app_icon parameter or"
<< LipstickNotification::HINT_IMAGE_PATH << "instead";
}

if (hint.compare(LipstickNotification::HINT_APP_ICON, Qt::CaseInsensitive) != 0 &&
hint.compare(LipstickNotification::HINT_TIMESTAMP, Qt::CaseInsensitive) != 0 &&
if (hint.compare(LipstickNotification::HINT_TIMESTAMP, Qt::CaseInsensitive) != 0 &&
hint.compare(LipstickNotification::HINT_PREVIEW_SUMMARY, Qt::CaseInsensitive) != 0 &&
hint.compare(LipstickNotification::HINT_PREVIEW_BODY, Qt::CaseInsensitive) != 0 &&
hint.compare(LipstickNotification::HINT_SUB_TEXT, Qt::CaseInsensitive) != 0 &&
Expand All @@ -482,6 +506,7 @@ QDBusArgument &operator<<(QDBusArgument &argument, const LipstickNotification &n
argument.beginStructure();
argument << notification.m_appName;
argument << notification.m_id;
argument << notification.m_appIcon;
argument << notification.m_summary;
argument << notification.m_body;
argument << notification.m_actions;
Expand All @@ -496,6 +521,7 @@ const QDBusArgument &operator>>(const QDBusArgument &argument, LipstickNotificat
argument.beginStructure();
argument >> notification.m_appName;
argument >> notification.m_id;
argument >> notification.m_appIcon;
argument >> notification.m_summary;
argument >> notification.m_body;
argument >> notification.m_actions;
Expand Down
13 changes: 10 additions & 3 deletions src/notifications/lipsticknotification.h
Expand Up @@ -30,11 +30,13 @@ class QDBusArgument;
class LIPSTICK_EXPORT LipstickNotification : public QObject
{
Q_OBJECT
Q_ENUMS(InformationOrigin)
Q_PROPERTY(QString appName READ appName CONSTANT)
Q_PROPERTY(QString explicitAppName READ explicitAppName CONSTANT)
Q_PROPERTY(QString disambiguatedAppName READ disambiguatedAppName CONSTANT)
Q_PROPERTY(uint id READ id CONSTANT)
Q_PROPERTY(QString appIcon READ appIcon NOTIFY appIconChanged)
Q_PROPERTY(int appIconOrigin READ appIconOrigin NOTIFY appIconOriginChanged)
Q_PROPERTY(QString summary READ summary NOTIFY summaryChanged)
Q_PROPERTY(QString body READ body NOTIFY bodyChanged)
Q_PROPERTY(QStringList actions READ actions CONSTANT)
Expand All @@ -57,6 +59,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject

public:
enum Urgency { Low = 0, Normal = 1, Critical = 2 };
enum InformationOrigin { ExplicitValue, CategoryValue, InferredValue };

//! Standard hint: The urgency level.
static const char *HINT_URGENCY;
Expand All @@ -82,9 +85,6 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
//! Standard hint: If set, override possible audible feedback sound.
static const char *HINT_SOUND_FILE;

//! Standard hint: Icon ID of the application sending the notification.
static const char *HINT_APP_ICON;

//! Nemo hint: Item count represented by the notification.
static const char *HINT_ITEM_COUNT;

Expand Down Expand Up @@ -182,6 +182,9 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject

//! Returns the icon ID of the application sending the notification
QString appIcon() const;
void setAppIcon(const QString &appIcon, int source = ExplicitValue);

int appIconOrigin() const;

//! Returns the summary text for the notification
QString summary() const;
Expand Down Expand Up @@ -301,6 +304,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject

//! Sent when the app icon has been modified
void appIconChanged();
void appIconOriginChanged();

//! Sent when the timestamp has changed
void timestampChanged();
Expand Down Expand Up @@ -345,6 +349,9 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
//! The ID of the notification
uint m_id;

QString m_appIcon;
int m_appIconOrigin = ExplicitValue;

//! Summary text for the notification
QString m_summary;

Expand Down
25 changes: 15 additions & 10 deletions src/notifications/notificationmanager.cpp
Expand Up @@ -199,7 +199,6 @@ QStringList NotificationManager::GetCapabilities()
<< "actions"
<< "persistence"
<< "sound"
<< LipstickNotification::HINT_APP_ICON
<< LipstickNotification::HINT_ITEM_COUNT
<< LipstickNotification::HINT_TIMESTAMP
<< LipstickNotification::HINT_PREVIEW_BODY
Expand Down Expand Up @@ -391,7 +390,7 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
notification->setAppName(pidProperties.first);
}
if (notification->appIcon().isEmpty() && !pidProperties.second.isEmpty()) {
hints_.insert(LipstickNotification::HINT_APP_ICON, pidProperties.second);
notification->setAppIcon(pidProperties.second, LipstickNotification::InferredValue);
}

// Use the summary and body as fallback values for previewSummary and previewBody.
Expand Down Expand Up @@ -626,7 +625,7 @@ void NotificationManager::applyCategoryDefinition(LipstickNotification *notifica
}
} else if (key == QString("app_icon")) {
if (notification->appIcon().isEmpty()) {
hints.insert(LipstickNotification::HINT_APP_ICON, value);
notification->setAppIcon(value, LipstickNotification::CategoryValue);
}
} else if (key == QString("summary")) {
if (notification->summary().isEmpty()) {
Expand Down Expand Up @@ -665,10 +664,10 @@ void NotificationManager::publish(const LipstickNotification *notification, uint
}

// Add the notification, its actions and its hints to the database
execSQL("INSERT INTO notifications VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
execSQL("INSERT INTO notifications VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
QVariantList() << id << notification->appName() << notification->appIcon() << notification->summary()
<< notification->body() << notification->expireTimeout() << notification->disambiguatedAppName()
<< notification->explicitAppName());
<< notification->explicitAppName() << notification->appIconOrigin());

// every other is identifier and every other the localized name for it
bool everySecond = false;
Expand Down Expand Up @@ -789,17 +788,19 @@ bool NotificationManager::checkTableValidity()
} else {
if (databaseVersion == 3) {
QSqlQuery query(*m_database);
if (query.exec("ALTER TABLE notifications ADD COLUMN explicit_app_name TEXT")) {
if (query.exec("ALTER TABLE notifications ADD COLUMN explicit_app_name TEXT")
&& query.exec("ALTER TABLE notifications ADD COLUMN app_icon_origin INTEGER")) {
qWarning() << "Extended notifications table";
} else {
qWarning() << "Failed to extend notifications table!";
qWarning() << "Failed to extend notifications table!" << query.lastError();
recreateNotificationsTable = true;
}

} else {
recreateNotificationsTable = !verifyTableColumns("notifications",
QStringList() << "id" << "app_name" << "app_icon" << "summary"
<< "body" << "expire_timeout" << "disambiguated_app_name" << "explicit_app_name");
<< "body" << "expire_timeout" << "disambiguated_app_name" << "explicit_app_name"
<< "app_icon_origin");
recreateActionsTable = !verifyTableColumns("actions", QStringList() << "id" << "action" << "display_name");
}

Expand All @@ -811,7 +812,7 @@ bool NotificationManager::checkTableValidity()
qWarning() << "Recreating notifications table";
result &= recreateTable("notifications", "id INTEGER PRIMARY KEY, app_name TEXT, app_icon TEXT, summary TEXT, "
"body TEXT, expire_timeout INTEGER, disambiguated_app_name TEXT, "
"explicit_app_name TEXT");
"explicit_app_name TEXT, app_icon_origin INTEGER");
}
if (recreateActionsTable) {
qWarning() << "Recreating actions table";
Expand Down Expand Up @@ -957,15 +958,18 @@ void NotificationManager::fetchData(bool update)
int notificationsTableExplicitAppNameFieldIndex = notificationsRecord.indexOf("explicit_app_name");
int notificationsTableDisambiguatedAppNameFieldIndex = notificationsRecord.indexOf("disambiguated_app_name");
int notificationsTableAppIconFieldIndex = notificationsRecord.indexOf("app_icon");
int notificationsTableAppIconOriginFieldIndex = notificationsRecord.indexOf("app_icon_origin");
int notificationsTableSummaryFieldIndex = notificationsRecord.indexOf("summary");
int notificationsTableBodyFieldIndex = notificationsRecord.indexOf("body");
int notificationsTableExpireTimeoutFieldIndex = notificationsRecord.indexOf("expire_timeout");

while (notificationsQuery.next()) {
const uint id = notificationsQuery.value(notificationsTableIdFieldIndex).toUInt();
QString appName = notificationsQuery.value(notificationsTableAppNameFieldIndex).toString();
QString explicitAppName = notificationsQuery.value(notificationsTableExplicitAppNameFieldIndex).toString();
QString disambiguatedAppName = notificationsQuery.value(notificationsTableDisambiguatedAppNameFieldIndex).toString();
QString appIcon = notificationsQuery.value(notificationsTableAppIconFieldIndex).toString();
int appIconOrigin = notificationsQuery.value(notificationsTableAppIconOriginFieldIndex).toInt();
QString summary = notificationsQuery.value(notificationsTableSummaryFieldIndex).toString();
QString body = notificationsQuery.value(notificationsTableBodyFieldIndex).toString();
int expireTimeout = notificationsQuery.value(notificationsTableExpireTimeoutFieldIndex).toInt();
Expand Down Expand Up @@ -996,8 +1000,9 @@ void NotificationManager::fetchData(bool update)
}

LipstickNotification *notification = new LipstickNotification(appName, explicitAppName, disambiguatedAppName,
id, appIcon, summary, body, notificationActions,
id, QString(), summary, body, notificationActions,
notificationHints, expireTimeout, this);
notification->setAppIcon(appIcon, appIconOrigin);
m_notifications.insert(id, notification);

if (id > m_previousNotificationID) {
Expand Down
3 changes: 1 addition & 2 deletions src/notifications/thermalnotifier.cpp
Expand Up @@ -66,14 +66,13 @@ void ThermalNotifier::publishTemperatureNotification(const QString &body)
NotificationManager *manager = NotificationManager::instance();

QVariantHash hints;
hints.insert(LipstickNotification::HINT_APP_ICON, "icon-system-warning");
hints.insert(LipstickNotification::HINT_URGENCY, LipstickNotification::Critical);
hints.insert(LipstickNotification::HINT_TRANSIENT, true);
hints.insert(LipstickNotification::HINT_FEEDBACK, "general_warning");

manager->Notify(manager->systemApplicationName(),
0,
QString(),
QLatin1String("icon-system-warning"),
QString(),
body,
QStringList(),
Expand Down
3 changes: 1 addition & 2 deletions src/shutdownscreen.cpp
Expand Up @@ -133,13 +133,12 @@ void ShutdownScreen::setUser(uint uid)
void ShutdownScreen::publishNotification(const QString &icon, const QString &feedback, const QString &body)
{
QVariantHash hints;
hints.insert(LipstickNotification::HINT_APP_ICON, icon);
hints.insert(LipstickNotification::HINT_URGENCY, LipstickNotification::Critical);
hints.insert(LipstickNotification::HINT_TRANSIENT, true);
hints.insert(LipstickNotification::HINT_FEEDBACK, feedback);

NotificationManager *manager = NotificationManager::instance();
manager->Notify(manager->systemApplicationName(), 0, QString(), QString(), body, QStringList(), hints, -1);
manager->Notify(manager->systemApplicationName(), 0, icon, QString(), body, QStringList(), hints, -1);
}

void ShutdownScreen::setShutdownMode(const QString &mode)
Expand Down

0 comments on commit e3d8367

Please sign in to comment.