GNOME Bugzilla – Bug 684620
Make fallback warning less annoying
Last modified: 2021-06-14 18:21:57 UTC
Created attachment 224974 [details] [review] Use libnotify to display fallback warning The fallback warning can get annoying, especially when using live systems. This is why Ubuntu has chosen to disable it, but at the risk of users wondering why the desktop looks different on a different machine. It is much less annoying to use a notification with libnotify instead, when available.
Just depend on libnotify. No point in ifdeffing this.
Created attachment 224989 [details] [review] Use libnotify to display fallback warning Well if you prefer it this way, it’s simpler of course.
Review of attachment 224989 [details] [review]: This could use designer input. ::: gnome-session/gsm-manager.c @@ +1371,3 @@ + notif = notify_notification_new (title, description, GSM_ICON_COMPUTER_FAIL); + notify_notification_set_timeout (notif, 15*1000); + notify_notification_add_action (notif, "link-click", link_text, NOTIFY_ACTION_CALLBACK (on_link_clicked), (gpointer) uri, NULL); Your cast here is subverting what the compiler was likely telling you - passing a const char * here is wrong since the string may have been freed. You need to g_strdup(). @@ +1372,3 @@ + notify_notification_set_timeout (notif, 15*1000); + notify_notification_add_action (notif, "link-click", link_text, NOTIFY_ACTION_CALLBACK (on_link_clicked), (gpointer) uri, NULL); + /* Give the notification daemon a chance to finish initialization */ Sigh =( Notifications popping up before the UI is initialized have always been a very embarassing part of GNOME 2. Oh well.
Created attachment 225016 [details] [review] Use libnotify to display fallback warning D’uh, I’m ashamed at the trivial mistake. Thanks for the review.
Review of attachment 225016 [details] [review]: The code looks OK to me, but this could still use some sort of designer sign off. Are you aiming for 3.6 with this? ::: gnome-session/gsm-manager.c @@ +1374,3 @@ + notify_notification_add_action (notif, "link-click", link_text, NOTIFY_ACTION_CALLBACK (on_link_clicked), g_strdup (uri), g_free); + /* Give the notification daemon a chance to finish initialization */ + g_timeout_add_seconds (2, (GSourceFunc) notification_show_timeout, (gpointer) notif); I don't understand why you have these casts here. Both the source func and the data match the expected prototype. Personally in my code I always match prototypes rather than casting, because it forces you to remember that source funcs have a gboolean return...refactoring some code that returns (void) is all too easy, and then you're in undefined behavior.
is the timeout for making sure the desktop is loaded or for making sure the notification daemon is loaded? If the first, gnome-session should know that more precisely already since it knows what phase of loading the session is in. If the latter, why is this necessary? Is there some race condition?
Do you have before and after screenshots?
(In reply to comment #5) > I don't understand why you have these casts here. Both the source func and the > data match the expected prototype. Just a very old (and bad) reflex. Will remove them. (In reply to comment #6) > is the timeout for making sure the desktop is loaded or for making sure the > notification daemon is loaded? > If the first, gnome-session should know that more precisely already since it > knows what phase of loading the session is in. If the latter, why is this > necessary? Is there some race condition? We are already in the RUNNING phase, so the desktop is already being loaded at this moment. The timeout is for the notification daemon, because it registers itself to the session manager before being able to answer DBus requests. I guess the bug could be fixed in notification-daemon itself.
Josselin, could you please create the requested screenshots above?
(In reply to comment #9) > Josselin, could you please create the requested screenshots above? Well, sorry if the question sounds dumb, but how do you do that now that the gdmflexiserver --xnest functionality is gone? The warnings appear at a time when you can’t do screenshots yet.
you can get gdmflexiserver --xnest like functionality by enabling XDMCP in gdm and doing Xephyr -query localhost :123 (though I'm not sure why screenshot keybinding isn't available for you)
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 of gnome-session, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/gnome-session/-/issues/ Thank you for your understanding and your help.