GNOME Bugzilla – Bug 665957
Notification daemon does not handle the image-path hint correctly
Last modified: 2021-07-05 14:31:43 UTC
The notification spec at http://developer.gnome.org/notification-spec/#icons-and-images says : The "app_icon" parameter and "image-path" hint should be either an URI (file:// is the only URI schema supported right now) or a name in a freedesktop.org-compliant icon theme (not a GTK+ stock ID). But if you set the image-path hint to a valid icon name, it triggers the following error : Error invoking GLib.filename_to_uri: The pathname 'audio-x-generic' is not an absolute path It looks like the code expects the hint to always contains a full path, which is not correct according to the spec: http://git.gnome.org/browse/gnome-shell/tree/js/ui/notificationDaemon.js#n136 and http://git.gnome.org/browse/gnome-shell/tree/js/ui/notificationDaemon.js#n366
"image-path" is arguably a very bad name for a parameter giving the icon name. ;-) Could you provide a patch, since you spotted where the problem is in the code? I guess checking whether image-path starts with "file://" should be enough.
(In reply to comment #1) > "image-path" is arguably a very bad name for a parameter giving the icon name. > ;-) Agreed, blame the spec writers :) > Could you provide a patch, since you spotted where the problem is in the code? > I guess checking whether image-path starts with "file://" should be enough. Sorry, I can read JavaScript, but I'm not comfortable writing it. I'm a strongly-typed kinda guy ;) And I have my hands full trying to get Banshee to play nice with GTK+ 3...
Created attachment 203283 [details] [review] notificationDaemon: Properly interpret image-path hint We were interpreting the image-path hint as an absolute path. In reality, the image-path hint specifies either a file: URI or an icon name. Weird, I know.
Review of attachment 203283 [details] [review]: ::: js/ui/notificationDaemon.js @@ +136,3 @@ } else if (hints['image-path']) { + try { + return textureCache.load_uri_async(GLib.filename_from_uri(hints['image-path'], null), size, size); Wouldn't it be cleaner to check for "file://" just like the code already does above? (I know, I could probably have written the two lines instead of commenting... :-)
Created attachment 203284 [details] [review] notificationDaemon: Properly interpret image-path hint We were interpreting the image-path hint as an absolute path. In reality, the image-path hint specifies either a file: URI or an icon name. Weird, I know.
Review of attachment 203284 [details] [review]: ::: js/ui/notificationDaemon.js @@ +111,3 @@ + + // Takes a filename, URI or an icon name. + _iconFromString: function(icon) { Don't you need to pass the size as an argument? Other than that, looks fine to me (but I didn't test it).
Created attachment 203786 [details] [review] notificationDaemon: Properly interpret image-path hint We were interpreting the image-path hint as an absolute path. In reality, the image-path hint specifies either a file: URI or an icon name. Weird, I know. Whoops
It seems the patch hasn't been committed, but I don't see why. Has it just fallen through the cracks ? Having this fixed would allow us to remove a TODO in Banshee: https://git.gnome.org/browse/banshee/tree/src/Extensions/Banshee.NotificationArea/Banshee.NotificationArea/NotificationAreaService.cs#n469
(In reply to comment #8) > It seems the patch hasn't been committed, but I don't see why. > Has it just fallen through the cracks ? Nobody reviewed it.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/ Thank you for your understanding and your help.