GNOME Bugzilla – Bug 702377
Add support for notifications
Last modified: 2013-09-15 02:28:55 UTC
Much like rhythmbox does, using all the fancy gnome-shell stuff.
Created attachment 246916 [details] [review] AlbumArtCache: cleanup the public interface and correct the filenames Almost always, the user of AlbumArtCache wants to lookup the image in cache or trigger a resolve operation and fetch from the network. Also, sometimes the API user is interested in the cached path too (for example to send to another app). Naturally, for that to work we need to ensure that PNG thumbnails end up with .png, not .jpeg. For us it works because we use gdk_pixbuf_new_from_stream(), which uses sniffing exclusively.
Created attachment 246917 [details] [review] Player: make it observable from the outside Add a public "playing" property, which wraps the Gst state in a high livel boolean, and add signals to monitor playing state and current track.
Created attachment 246918 [details] [review] Add support for notifications Add a NotificationManager object that watches the Player and keeps a resident notification to control the application.
Cannot apply the third patch: Applying: Add support for notifications fatal: sha1 information is lacking or useless (src/application.js). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Add support for notifications Please rebase on the latest master
Review of attachment 246918 [details] [review]: Prints non-critical error (on gnome 3.8 at least) (gnome-music:18678): Gjs-WARNING **: JS ERROR: Exception in callback for signal: playing-changed: Error: Too few arguments to method Notify.add_action expected 4 got 3 NotificationManager<._setActions@/opt/gnome/share/gnome-music/notifications.js:124 wrapper@/opt/gnome/share/gjs-1.0/lang.js:213 NotificationManager<._init/<@/opt/gnome/share/gnome-music/notifications.js:51 _emit@/opt/gnome/share/gjs-1.0/signals.js:124 Player<._init/<@/opt/gnome/share/gnome-music/player.js:75 main@/opt/gnome/share/gnome-music/main.js:29 start@/opt/gnome/share/gnome-music/package.js:154 @/opt/gnome/bin/gnome-music:3
I forgot to mention this depends on libnotify bug 702390.
Created attachment 247159 [details] [review] Add support for notifications Add a NotificationManager object that watches the Player and keeps a resident notification to control the application.
Thanks, looks good, waiting for bug 702390 to be resolved to commit this.
The following fixes have been pushed: de7293a Player: make it observable from the outside 2c93196 AlbumArtCache: cleanup the public interface and correct the filenames
Created attachment 247439 [details] [review] Player: make it observable from the outside Add a public "playing" property, which wraps the Gst state in a high livel boolean, and add signals to monitor playing state and current track.
Created attachment 247440 [details] [review] AlbumArtCache: cleanup the public interface and correct the filenames Almost always, the user of AlbumArtCache wants to lookup the image in cache or trigger a resolve operation and fetch from the network. Also, sometimes the API user is interested in the cached path too (for example to send to another app). Naturally, for that to work we need to ensure that PNG thumbnails end up with .png, not .jpeg. For us it works because we use gdk_pixbuf_new_from_stream(), which uses sniffing exclusively.
Created attachment 248391 [details] [review] AlbumArtCache: don't drop requests on the floor on failure Previously, we would not call the callback unless the album art was successfully retrieved, but that would leave callers waiting forever on failure. Instead, invoke the callback with a null pixbuf. I had this as an amend to the previous AAC patch. It's needed by the notification manager, which otherwise shows nothing for albums without art.
These patches need to be ported to python.
Comment on attachment 247159 [details] [review] Add support for notifications Port to Python is needed.
Comment on attachment 248391 [details] [review] AlbumArtCache: don't drop requests on the floor on failure This is already implemented in current master.
Created attachment 253755 [details] [review] Add support for notifications Add a NotificationManager object that watches the Player and keeps a resident notification to control the application.
(In reply to comment #16) > Created an attachment (id=253755) [details] [review] > Add support for notifications > > Add a NotificationManager object that watches the Player and > keeps a resident notification to control the application. Need to use rtl variants of icons when using RTL languages, for example: https://git.gnome.org/browse/rhythmbox/commit/?id=eb4641a127828a0fa567eb19c26c66ff7f3b2f52 and https://git.gnome.org/browse/gnome-music/commit/?id=cf8c8c80ce400220b23f932198340fa3f1e6e307
Review of attachment 253755 [details] [review]: I tested this patch and seems alright. Vadim accepted it for commit. But because of the UI and String Freeze, this is on hold and is currently residing in a branch in out GitHub mirror at: https://github.com/gnome-prototypes-team/gnome-music/commits/notifications Fixes for RTL is still not there, but we will ensure that this issue will be fixed before merging. It will merged along with some fixes after the release team approves the following UI Freeze Break: https://mail.gnome.org/archives/release-team/2013-September/msg00028.html The String Freeze Break is already accepted by Piotr Drąg and Petr Kovar.
Comment on attachment 253755 [details] [review] Add support for notifications Committed as a4c8649ce. RTL will be fixed later. Accepted for UI freeze break in https://mail.gnome.org/archives/release-team/2013-September/msg00088.html
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report (and patches :D).
*** Bug 708065 has been marked as a duplicate of this bug. ***