Skip to content

Commit 361526c

Browse files
mtwebstermtwebster
committed
notificationDaemon.js: Cleanup notification app_icon and icon-path
handling. - (Readability) Use appIcon instead of icon everywhere to refer to what should be a url or icon-name for the application that sent the notificataion. - Sanitize app_icon and icon-path values when the notificatioin is first received. Per the fdo spec, only file uris and icon-names are valid. - Fully support icon-path in _notifyForSource() (icon names weren't being handled). - Catch and return _notifyForSource() errors to the dbus invocation. - Only show an additional image (based on icon-data or -path) if the uri or icon is different from app_icon. Otherwise, both a normal smaller icon and duplicate large one will be shown, which doesn't look nice. The last change is based on an issue with Thunar (as of 4.18.8 at least) when using libnotify 0.86: - thunar_notify_show() creates a new notification using notify_notification_new() providing a device icon. This is set as the 'app-icon' property internally to libnotify. - later, thunar_notify_progress calls notify_notification_update() with that same icon, only now it's set as the 'image-path' hint (as well as icon_name internally). (I think in previous libnotify versions, this argument would be more or less ignored internally), and the only way to use image-path was to set the hint directly). Fixes #13026 Thunar was freezing in Cinnamon because of item 3 above - _notifyForsource() was assuming icon-path would be a path or uri, when only a uri or icon name was supported. The g_filename_to_uri() error wasn't being caught, and the dbus invocation was never returning. ref: https://gitlab.gnome.org/GNOME/libnotify/-/commit/f5842adeffdbb8675484b89867938249a65485d9 https://bugzilla.redhat.com/show_bug.cgi?id=2372896
1 parent a5ca299 commit 361526c

File tree

1 file changed

+47
-38
lines changed

1 file changed

+47
-38
lines changed

‎js/ui/notificationDaemon.js

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,8 @@ NotificationDaemon.prototype = {
133133
},
134134

135135
// Create an icon for a notification from icon string/path.
136-
_iconForNotificationData: function(icon, hints, size) {
136+
_iconForNotificationData: function(appIcon, hints, size) {
137137
let textureCache = St.TextureCache.get_default();
138-
139138
// If an icon is not specified, we use 'image-data' or 'image-path' hint for an icon
140139
// and don't show a large image. There are currently many applications that use
141140
// notify_notification_set_icon_from_pixbuf() from libnotify, which in turn sets
@@ -144,16 +143,14 @@ NotificationDaemon.prototype = {
144143
// So the logic here does the right thing for this case. If both an icon and either
145144
// one of 'image-data' or 'image-path' are specified, we show both an icon and
146145
// a large image.
147-
if (icon) {
148-
if (icon.substr(0, 7) == 'file://')
149-
return textureCache.load_uri_async(icon, size, size);
150-
else if (icon[0] == '/') {
151-
let uri = GLib.filename_to_uri(icon, null);
152-
return textureCache.load_uri_async(uri, size, size);
146+
if (appIcon) {
147+
if (appIcon.startsWith("file://")) {
148+
return textureCache.load_uri_async(appIcon, size, size);
153149
} else {
154-
// If an icon name is specified, try to load it
155-
// in symbolic. If that fails, St reverts to fullcolor anyway
156-
return new St.Icon({ icon_name: icon,
150+
// Cinnamon prefers symbolic icons due to theming. If an icon
151+
// name is specified, try to load it in symbolic. If that fails,
152+
// St reverts to fullcolor anyway.
153+
return new St.Icon({ icon_name: appIcon,
157154
icon_type: St.IconType.SYMBOLIC,
158155
icon_size: size });
159156
}
@@ -162,17 +159,12 @@ NotificationDaemon.prototype = {
162159
bitsPerSample, nChannels, data] = hints['image-data'];
163160
return textureCache.load_from_raw(data, hasAlpha, width, height, rowStride, size);
164161
} else if (hints['image-path']) {
165-
let path = hints['image-path'];
166-
if (GLib.path_is_absolute (path)) {
167-
return textureCache.load_uri_async(GLib.filename_to_uri(path, null), size, size);
162+
let uri_or_icon_name = hints['image-path'];
163+
if (uri_or_icon_name.startsWith("file://")) {
164+
return textureCache.load_uri_async(uri_or_icon_name, size, size);
168165
} else {
169-
let icon_type = St.IconType.FULLCOLOR;
170-
if (path.search("-symbolic") != -1) {
171-
icon_type = St.IconType.SYMBOLIC;
172-
}
173-
174-
return new St.Icon({ icon_name: path,
175-
icon_type: icon_type,
166+
return new St.Icon({ icon_name: uri_or_icon_name,
167+
icon_type: St.IconType.FULLCOLOR,
176168
icon_size: size });
177169
}
178170
} else {
@@ -285,7 +277,7 @@ NotificationDaemon.prototype = {
285277

286278
// Sends a notification to the notification daemon. Returns the id allocated to the notification.
287279
NotifyAsync: function(params, invocation) {
288-
let [appName, replacesId, icon, summary, body, actions, hints, timeout] = params;
280+
let [appName, replacesId, appIcon, summary, body, actions, hints, timeout] = params;
289281
let id;
290282

291283
for (let hint in hints) {
@@ -318,10 +310,19 @@ NotificationDaemon.prototype = {
318310
// early versions of the spec; 'icon_data' should only be used if 'image-path' is not available
319311
hints['image-data'] = hints['icon_data'];
320312

313+
// Spec requires app_icon and image-path to be a file uri or icon name
314+
// https://specifications.freedesktop.org/notification-spec/latest/icons-and-images.html#icons-and-images-formats
315+
if (appIcon && GLib.path_is_absolute(appIcon)) {
316+
appIcon = GLib.filename_to_uri(appIcon, null);
317+
}
318+
if (hints['image-path'] && GLib.path_is_absolute(hints['image-path'])) {
319+
hints['image-path'] = GLib.filename_to_uri(hints['image-path'], null);
320+
}
321+
321322
hints['suppress-sound'] = hints.maybeGet('suppress-sound') == true;
322323

323324
let ndata = { appName: appName,
324-
icon: icon,
325+
appIcon: appIcon,
325326
summary: summary,
326327
body: body,
327328
actions: actions,
@@ -367,8 +368,12 @@ NotificationDaemon.prototype = {
367368
let source = this._getSource(appName, pid, ndata, sender, null);
368369

369370
if (source) {
370-
this._notifyForSource(source, ndata);
371-
return invocation.return_value(GLib.Variant.new('(u)', [id]));
371+
try {
372+
this._notifyForSource(source, ndata);
373+
return invocation.return_value(GLib.Variant.new('(u)', [id]));
374+
} catch (e) {
375+
return invocation.return_gerror(e);
376+
}
372377
}
373378

374379
if (replacesId) {
@@ -413,11 +418,11 @@ NotificationDaemon.prototype = {
413418
},
414419

415420
_notifyForSource: function(source, ndata) {
416-
let [id, icon, summary, body, actions, hints, notification, timeout, expires] =
417-
[ndata.id, ndata.icon, ndata.summary, ndata.body,
421+
let [id, appIcon, summary, body, actions, hints, notification, timeout, expires] =
422+
[ndata.id, ndata.appIcon, ndata.summary, ndata.body,
418423
ndata.actions, ndata.hints, ndata.notification, ndata.timeout, ndata.expires];
419424

420-
let iconActor = this._iconForNotificationData(icon, hints, source.ICON_SIZE);
425+
let iconActor = this._iconForNotificationData(appIcon, hints, source.ICON_SIZE);
421426

422427
if (notification == null) { // Create a new notification!
423428
notification = new MessageTray.Notification(source, summary, body,
@@ -463,23 +468,27 @@ NotificationDaemon.prototype = {
463468
silent: hints['suppress-sound'] });
464469
}
465470

466-
// We only display a large image if an icon is also specified.
467-
if (icon && (hints['image-data'] || hints['image-path'])) {
471+
// We only display a large image if an icon is also specified in the hints AND it is
472+
// **not** the same icon as appIcon.
473+
if (appIcon && (hints['image-data'] || (hints['image-path'] && hints['image-path'] !== appIcon))) {
468474
let image = null;
469475
if (hints['image-data']) {
470476
let [width, height, rowStride, hasAlpha,
471477
bitsPerSample, nChannels, data] = hints['image-data'];
472478
image = St.TextureCache.get_default().load_from_raw(data, hasAlpha,
473479
width, height, rowStride, notification.IMAGE_SIZE);
474480
} else if (hints['image-path']) {
475-
let image_path = hints['image-path'];
476-
// Convert filepaths to file URIs but leave file URIs untouched
477-
if (image_path.substr(0, 7) != 'file://') {
478-
image_path = GLib.filename_to_uri(image_path, null);
481+
let uri_or_icon_name = hints['image-path'];
482+
483+
if (uri_or_icon_name.startsWith("file://")) {
484+
image = St.TextureCache.get_default().load_uri_async(uri_or_icon_name,
485+
notification.IMAGE_SIZE,
486+
notification.IMAGE_SIZE);
487+
} else {
488+
image = new St.Icon({ icon_name: uri_or_icon_name,
489+
icon_type: St.IconType.FULLCOLOR,
490+
icon_size: notification.IMAGE_SIZE });
479491
}
480-
image = St.TextureCache.get_default().load_uri_async(image_path,
481-
notification.IMAGE_SIZE,
482-
notification.IMAGE_SIZE);
483492
}
484493
notification.setImage(image);
485494
} else {
@@ -516,7 +525,7 @@ NotificationDaemon.prototype = {
516525
// of the 'transient' hint with hints['transient'] rather than hints.transient
517526
notification.setTransient(hints.maybeGet('transient') == true);
518527

519-
let sourceIconActor = source.useNotificationIcon ? this._iconForNotificationData(icon, hints, source.ICON_SIZE) : null;
528+
let sourceIconActor = source.useNotificationIcon ? this._iconForNotificationData(appIcon, hints, source.ICON_SIZE) : null;
520529
source.processNotification(notification, sourceIconActor);
521530
},
522531

0 commit comments

Comments
 (0)