Merge lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix- into lp:~wingpanel-devs/wingpanel-indicator-notifications/wingpanel-indicator-notifications

[Giuliano Fiorotto]

This branch aims to fix bug #1594227 to open applications whose notifications action is set to default by making the app the active window if it is already opened or open it otherwise.

[Adam Bieńkowski (donadigo)]

Is the change with removing the "!" intentional? This will make it so that even if the action is launched the app will be activated, which in this case, should be handeled by the app itself, not the indicator / window manager.

review: Needs Information (code)

[Giuliano Fiorotto (mr-fiorotto)]

> Is the change with removing the "!" intentional? This will make it so that
> even if the action is launched the app will be activated, which in this case,
> should be handeled by the app itself, not the indicator / window manager.

The removal of ! is intentional because, if i'm not wrong, every notification that has default as the specified key for actions should open the app, or should focus it if already opened. No? From gnome specs: "The default action (usually invoked my clicking the notification) should have a key named 'default'. The name can be anything, though implementations are free not to display it." and from what i see, the method run_default_action check if the action is default, then if yes it should activate the program. Please correct me if i'm wrong.

[Adam Bieńkowski (donadigo)]

mr-fiorotto: here is the code that handles this from gala, the window manager of elementary OS:
http://bazaar.launchpad.net/~gala-dev/gala/trunk/view/head:/plugins/notify/NormalNotification.vala#L285

As you can see, first gala checks if there's a default action, if there is, it calls action_invoked and *returns* from the activate () method, there is no activating the window after that. However if the action wasn't found, it gets the window by PID and activates it. That should be also the case within the indicator.

[Danielle Foré (danrabbit)]

Hey Giuliano, just following up to say that this fixed was eventually merged on GitHub and there is a bounty for you to claim here: https://www.bountysource.com/issues/36132032-clicking-a-notification-should-open-the-app

Unmerged revisions

134. By Giuliano Fiorotto on 2017-01-19

trying-to-fix-#1594227

=== modified file 'src/Widgets/NotificationsList.vala'
--- src/Widgets/NotificationsList.vala	2016-12-07 18:36:42 +0000
+++ src/Widgets/NotificationsList.vala	2017-01-19 21:54:15 +0000
@@ -197,7 +197,7 @@
         } else if (row is NotificationEntry) {
             var notification_entry = (NotificationEntry)row;
 
-            if (!notification_entry.notification.run_default_action ()) {
+            if (notification_entry.notification.run_default_action ()) {
                 close = focus_notification_app (notification_entry.notification.get_app_window (),
                                                 notification_entry.notification.app_info);
             }
@@ -216,7 +216,7 @@
 
     private bool focus_notification_app (Wnck.Window? app_window, AppInfo? app_info) {
         if (app_window != null) {
-            app_window.unminimize (Gtk.get_current_event_time ());
+            app_window.activate (Gtk.get_current_event_time ());
             return true;
         } else if (app_info != null) {
             try {