Created attachment 285879 [details] [review]
search-provider: use default web browser

The search provider should be useful even if the user's default web
browser isn't epiphany. This allows distros that are hesitant to ship
epiphany to package the search provider separately from the desktop
file. Use gvfs-open to open the URI in the default browser, and fall
back to epiphany if it's not installed.

I haven't figured out how to test this. If I edit /usr/share/dbus-1/services
and replace the Exec line with the path to the search provider in my jhbuild
install prefix, it doesn't work at all....

Review of attachment 285879 [details] [review]:

::: src/ephy-search-provider.c
@@ +271,3 @@
+
+  /* Attempt to display results in the user's default browser. */
+  str = g_strdup_printf ("gvfs-open %s", uri);

Instead of spawning gvfs-open, we can use GAppInfo API, and we don't depend on gvfs. We can get the default application to handle HTML and supporing URIs (it should be something like g_app_info_get_default_for_type ("text/html", TRUE)) and then create GdkAppLaunchContext (that would also fix the TODO: Handle the timestamp if you pass the timestamp to the launch context). And finally you only need to call g_app_info_launch_uris(). That will spawn the process for you, or will 'activate' the application via DBus if supported.

Created attachment 285918 [details] [review]
search-provider: launch results in default browser

This way, the search provider will be semi-useful even if the Epiphany
desktop file is not installed.

I haven't figured out how to test this. If I edit /usr/share/dbus-1/services
and replace the Exec line with the path to the search provider in my jhbuild
install prefix, it doesn't work at all....

Created attachment 285919 [details] [review]
search-provider: launch results in default browser

Fix bogus bug reference

Review of attachment 285919 [details] [review]:

::: src/ephy-search-provider.c
@@ +269,3 @@
+  GdkDisplay *display;
+  GdkAppLaunchContext *context;
+  GError *error;

This should be initialized to NULL.

@@ +278,3 @@
+  gdk_app_launch_context_set_timestamp (context, timestamp);
+
+  if (!g_app_info_launch_default_for_uri (uri, G_APP_LAUNCH_CONTEXT (context), &error)) {

I think launch_default_for_uri() also depends on gvfs, we don't need to guess the content type based on the uri, we already know we want th the default application to show HTML and that can handle URIs. That's why I proposed to use g_app_info_get_default_for_type() + g_app_info_launch_uris(), otherwise gtk_show_uri() would be even simpler

Created attachment 285952 [details] [review]
search provider: open result in default browser

Comment on attachment 285952 [details] [review]
search provider: open result in default browser

Thanks.

Attachment 285952 [details] pushed as faaa23c - search provider: open result in default browser

(In reply to comment #3)
> I haven't figured out how to test this. If I edit /usr/share/dbus-1/services
> and replace the Exec line with the path to the search provider in my jhbuild
> install prefix, it doesn't work at all....

Any hints on how to test search provider changes? I'm getting reports on IRC that launching results is broken, and I assume this commit is to blame.

Created attachment 286810 [details] [review]
Revert "search provider: open result in default browser"

This reverts commit faaa23cb8956336fcd16258fd74ab9f69c01e438.

Comment on attachment 286810 [details] [review]
Revert "search provider: open result in default browser"

Attachment 286810 [details] pushed as 1a058ad - Revert "search provider: open result in default browser"

(In reply to comment #9)
> (In reply to comment #3)
> > I haven't figured out how to test this. If I edit /usr/share/dbus-1/services
> > and replace the Exec line with the path to the search provider in my jhbuild
> > install prefix, it doesn't work at all....
> 
> Any hints on how to test search provider changes? I'm getting reports on IRC
> that launching results is broken, and I assume this commit is to blame.

The problem is, the search provider is a GApplication, not a GtkApplication, so GDK is not initialized and gdk_display_get_default() returns NULL.

OK, that makes sense. Thanks for debugging, Giovanni.

In other news, I finally figured out how to test changes to the search provider. In /usr/share/dbus-1/services/org.gnome.Epiphany.service, I have to change the Name line, not just the Exec line, since that changed in 3.12:

[D-BUS Service]
#Name=org.gnome.Epiphany
#Exec=/usr/bin/epiphany --headless-mode
Name=org.gnome.EpiphanySearchProvider
Exec=/home/mcatanzaro/jhbuild/install/libexec/epiphany-search-provider

All of which seems obvious in retrospect. Alas.

Anyway, Matthias requested that search providers not launch different applications. Maybe this makes sense, especially since Epiphany's is primarily useful for searching Epiphany's history and bookmarks, and the only way to change the search engine is Epiphany's preferences dialog. I don't really care either way; launching another browser seems harmless to me.

*** Bug 753774 has been marked as a duplicate of this bug. ***

I think the verdict on this (which never made it onto Bugzilla) was that search providers should not launch unrelated applications.