Merge lp:~kalikiana/midori/alwaysnotify into lp:midori

[Cris Dywan]

Make libnotify mandatory except on Windows

Make libnotify mandatory except for now on Windows

[Cris Dywan (kalikiana)]

A very interesting discussion the other day showed that some people may disable libnotify to "customize" Midori's behavior. I think this is misguided and apart from something like openbox libnotify is very unusual to not have available these days.
I'm leaving an exception for Windows for now because we don't have the same expectations there and at least currently don't think it's typical to have notifications there.

[Cris Dywan (kalikiana)]

Consider https://code.launchpad.net/~kalikiana/midori/manydowns/+merge/166483 for solving some of the behavior aspects people might've been trying to work-around.

[André Stösel (ivaldi)]

Works fine with Linux, but to be sure it would be nice if Paweł Forysiuk could review it (windows), too.

(Although the changes are really obviously - it's probably unnecessary ;)

review: Approve

[Paweł Forysiuk (tuxator)]

Seems to work fine on win32

review: Approve

[gue5t gue5t (gue5t)]

In my opinion it's perfectly sane for users to want some programs to not produce notifications. It's also sane for non-Windows systems to not have libnotify installed, and given that libnotify does not provide essential functionality I don't see why it should be made a hard-dependency. However, I do agree that ./configure is not the right place for users to configure whether on typical desktop systems whether they would like to see notifications from Midori. We should instead have a preference to control notifications.

Removing code that manually calls notify-send is a very good thing. Executing random system binaries is awful and Midori built on a system which wants notification features should depend on the requisite software, i.e. libnotify.

review: Needs Fixing

[Cris Dywan (kalikiana)]

> It's also sane for non-Windows systems to not have libnotify installed

I'm very doubtful here. All DEs roughly following freedesktop.org use notifications. It's also been proposed for inclusion with GTK+ which by itself says something.

> …libnotify does not provide essential functionality…

I disagree with this assumption and others voiced concerns that for example "launcher created" may not show up without libnotify. It is expected in the typical case, except custom kiosk setups and the like which may intentionally block the notification daemon.

I'll file a bug to re-consider the Windows case. I simply didn't want to block on it.

=== modified file 'README'
--- README	2013-05-16 21:25:22 +0000
+++ README	2013-05-30 11:15:49 +0000
@@ -12,9 +12,9 @@
 * Customizable interface, extensions written in C and Vala.
 
 Requirements: GLib 2.22, GTK+ 2.16, WebkitGTK+ 1.1.17, libXML2,
-              libsoup 2.27.90, sqlite 3.0, Vala 0.14
+              libsoup 2.27.90, sqlite 3.0, Vala 0.14, libnotify
 
-Optional: GTK+ 3.0, Unique 0.9, libnotify, gcr, Granite 0.2, WebKit2GTK+ 1.11.91/ 2.0.0
+Optional: GTK+ 3.0, Unique 0.9, gcr, Granite 0.2, WebKit2GTK+ 1.11.91/ 2.0.0
 
 For installation instructions read INSTALL.
 
 
=== modified file 'midori/midori-app.c'
--- midori/midori-app.c	2013-05-19 09:33:02 +0000
+++ midori/midori-app.c	2013-05-30 11:15:49 +0000
@@ -76,10 +76,6 @@
 
     MidoriBrowser* browser;
     MidoriAppInstance instance;
-
-    #if !HAVE_LIBNOTIFY
-    gchar* program_notify_send;
-    #endif
 };
 
 static gchar* app_name = NULL;
@@ -795,8 +791,6 @@
 
     #if HAVE_LIBNOTIFY
     notify_init (PACKAGE_NAME);
-    #else
-    app->program_notify_send = g_find_program_in_path ("notify-send");
     #endif
 }
 
@@ -824,8 +818,6 @@
     #if HAVE_LIBNOTIFY
     if (notify_is_initted ())
         notify_uninit ();
-    #else
-        katze_assign (app->program_notify_send, NULL);
     #endif
 
     G_OBJECT_CLASS (midori_app_parent_class)->finalize (object);
@@ -1312,21 +1304,6 @@
         notify_notification_show (note, NULL);
         g_object_unref (note);
     }
-    #else
-    /* Fall back to the command line program "notify-send" */
-    if (app->program_notify_send)
-    {
-        gchar* msgq = g_shell_quote (message);
-        gchar* titleq = g_shell_quote (title);
-        gchar* command = g_strdup_printf ("%s -i midori %s %s",
-            app->program_notify_send, titleq, msgq);
-
-        g_spawn_command_line_async (command, NULL);
-
-        g_free (titleq);
-        g_free (msgq);
-        g_free (command);
-    }
     #endif
 }
 
 
=== modified file 'wscript'
--- wscript	2013-05-22 20:17:41 +0000
+++ wscript	2013-05-30 11:15:49 +0000
@@ -193,12 +193,11 @@
             uselib_store=var, errmsg=name + ver_str + ' not found')][have])
         return have
 
-    if option_enabled ('libnotify'):
-        if not check_pkg ('libnotify', mandatory=False):
-            option_checkfatal ('libnotify', 'notifications')
-    else:
+    if is_win32 (os.environ):
         conf.define ('LIBNOTIFY_VERSION', 'No')
         conf.check_message_custom ('libnotify', '', 'disabled')
+    else:
+        check_pkg ('libnotify', mandatory=True)
     conf.define ('HAVE_LIBNOTIFY', [0,1][conf.env['LIBNOTIFY_VERSION'] != 'No'])
 
     if option_enabled ('granite'):
@@ -419,7 +418,6 @@
 
     group = opt.add_option_group ('Optional features', '')
     add_enable_option ('unique', 'single instance support', group, disable=is_win32 (os.environ))
-    add_enable_option ('libnotify', 'notification support', group)
     add_enable_option ('granite', 'new notebook, pop-overs', group, disable=True)
     add_enable_option ('addons', 'building of extensions', group)
     add_enable_option ('tests', 'install tests', group, disable=True)