[epiphany/mcatanzaro/file-launching: 1/5] Simplify and fix opening files
- From: Michael Catanzaro <mcatanzaro src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [epiphany/mcatanzaro/file-launching: 1/5] Simplify and fix opening files
- Date: Tue, 8 Jan 2019 03:41:39 +0000 (UTC)
commit 1791a6518736cf9a686d8d31e029cd651f04e64d
Author: Michael Catanzaro <mcatanzaro igalia com>
Date: Mon Jan 7 20:27:34 2019 -0600
Simplify and fix opening files
Opening files regressed in 7c8f19af448d42cb498bd729f498f49ebcd554c6.
EphyDownload always passes NULL for MIME type, so we skip all the code
for launching applications. Oops.
Instead of reverting this commit, let's delete all the code! We can
simplify file launching in several ways:
(1) ephy_embed_shell_launch_handler() is no longer needed at all. It's a
a wrapper around ephy_file_launch_handler() that just checks to ensure
Epiphany is not launching itself, to avoid recursive launching. This is
no longer needed because we no longer open files automatically under any
circumstances, so recursive launching is no longer possible (unless the
user manually opens the same file again and again for fun or something).
Remove this function altogether. EphyDownload can use
ephy_file_launch_handler() directly instead.
(2) Delete the test for ephy_embed_shell_launch_handler(), since it only
tests to ensure we avoid recursive launching, which is, again, no longer
a problem.
(3) ephy_file_launch_handler() doesn't actually need the MIME type for
anything, except for use by ephy_file_browse_to(). So get rid of it from
all associated functions. Reimplement ephy_file_browse_to() without it.
(The MIME type used to be important for making sure it's safe to
automatically open a file, but we never do that anymore.) A mistake
regarding the MIME types is what introduced the bug that triggered this
investigation, so eliminating it altogether seems worth it, especially
when it's only used in the corner case of ephy_file_browse_to().
(4) Make ephy_file_launch_handler() automatically launch via URI handler
when running in flatpak. This way, higher-level code no longer needs to
decide between using ephy_file_launch_handler() and
ephy_file_launch_file_via_uri_handler() based on whether flatpak is in
use.
(5) ephy_file_launch_file_via_uri_handler() and
ephy_file_launch_via_uri_handler() no longer need to be public, and can
be compressed into one function. ephy_file_launch_application also no
longer needs to be public.
(6) Add assertions to ephy_file_browse_to(),
ephy_file_launch_desktop_file(), and
ephy_file_open_uri_in_default_browser() to ensure they are never called
when running in flatpak, since it's impossible to make them work
(because the only way to open files in flatpak is via the OpenURI
portal, which gives the user, not the application, the choice of which
application to open). It's already impossible for any of these functions
to be called inside flatpak. (Hopefully! :)
I tried to split this up into separate commits, but it got rather
complicated, so here it is as an all-in-one.
Note this consequently fixes opening downloaded files in flatpak, albeit
not in the simplest-possible way.
embed/ephy-download.c | 3 +-
embed/ephy-embed-shell.c | 47 ---------------
embed/ephy-embed-shell.h | 4 --
lib/ephy-file-helpers.c | 130 +++++++++++++++++++-----------------------
lib/ephy-file-helpers.h | 11 +---
src/prefs-dialog.c | 3 +-
src/window-commands.c | 4 +-
tests/ephy-embed-shell-test.c | 21 -------
8 files changed, 64 insertions(+), 159 deletions(-)
---
diff --git a/embed/ephy-download.c b/embed/ephy-download.c
index d3118822f..e348ec0b9 100644
--- a/embed/ephy-download.c
+++ b/embed/ephy-download.c
@@ -444,8 +444,7 @@ ephy_download_do_download_action (EphyDownload *download,
break;
case EPHY_DOWNLOAD_ACTION_OPEN:
LOG ("ephy_download_do_download_action: open");
- ret = ephy_embed_shell_launch_handler (ephy_embed_shell_get_default (),
- destination, NULL, user_time);
+ ret = ephy_file_launch_handler (destination, user_time);
if (!ret)
ret = ephy_file_browse_to (destination, user_time);
break;
diff --git a/embed/ephy-embed-shell.c b/embed/ephy-embed-shell.c
index 02aedc24a..9d2a00578 100644
--- a/embed/ephy-embed-shell.c
+++ b/embed/ephy-embed-shell.c
@@ -1717,53 +1717,6 @@ ephy_embed_shell_get_mode (EphyEmbedShell *shell)
return priv->mode;
}
-/**
- * ephy_embed_shell_launch_handler:
- * @shell: an #EphyEmbedShell
- * @file: a #GFile to open
- * @mime_type: the mime type of @file or %NULL
- * @user_time: user time to prevent focus stealing
- *
- * Tries to open @file with the right application, making sure we will
- * not call ourselves in the process. This is needed to avoid
- * potential infinite loops when opening unknown file types.
- *
- * Returns: %TRUE on success
- **/
-gboolean
-ephy_embed_shell_launch_handler (EphyEmbedShell *shell,
- GFile *file,
- const char *mime_type,
- guint32 user_time)
-{
- GAppInfo *app;
- GList *list = NULL;
- gboolean ret = FALSE;
-
- g_assert (EPHY_IS_EMBED_SHELL (shell));
- g_assert (file);
-
- if (mime_type == NULL)
- return ret;
-
- if (ephy_is_running_inside_flatpak ()) {
- return ephy_file_launch_file_via_uri_handler (file);
- }
-
- app = ephy_file_launcher_get_app_info_for_file (file, mime_type);
-
- /* Do not allow recursive calls into the browser, they can lead to
- * infinite loops and they should never happen anyway. */
- if (!app || g_strcmp0 (g_app_info_get_id (app), "org.gnome.Epiphany.desktop") == 0)
- return ret;
-
- list = g_list_append (list, file);
- ret = ephy_file_launch_application (app, list, user_time, NULL);
- g_list_free (list);
-
- return ret;
-}
-
/**
* ephy_embed_shell_clear_cache:
* @shell: an #EphyEmbedShell
diff --git a/embed/ephy-embed-shell.h b/embed/ephy-embed-shell.h
index 3319dfcf1..4d9a351b6 100644
--- a/embed/ephy-embed-shell.h
+++ b/embed/ephy-embed-shell.h
@@ -72,10 +72,6 @@ void ephy_embed_shell_set_print_settings (EphyEmbedShell
GtkPrintSettings *settings);
GtkPrintSettings *ephy_embed_shell_get_print_settings (EphyEmbedShell *shell);
EphyEmbedShellMode ephy_embed_shell_get_mode (EphyEmbedShell *shell);
-gboolean ephy_embed_shell_launch_handler (EphyEmbedShell *shell,
- GFile *file,
- const char *mime_type,
- guint32 user_time);
void ephy_embed_shell_clear_cache (EphyEmbedShell *shell);
void ephy_embed_shell_set_thumbnail_path (EphyEmbedShell *shell,
const char *url,
diff --git a/lib/ephy-file-helpers.c b/lib/ephy-file-helpers.c
index 65313b915..89360129f 100644
--- a/lib/ephy-file-helpers.c
+++ b/lib/ephy-file-helpers.c
@@ -553,7 +553,7 @@ ephy_file_delete_on_exit (GFile *file)
*
* Returns: %TRUE if g_app_info_launch() succeeded
**/
-gboolean
+static gboolean
ephy_file_launch_application (GAppInfo *app,
GList *list,
guint32 user_time,
@@ -606,6 +606,11 @@ ephy_file_launch_desktop_file (const char *filename,
GList *list = NULL;
gboolean ret;
+ /* This is impossible to implement inside flatpak. Higher layers must
+ * ensure we don't get here.
+ */
+ g_assert (!ephy_is_running_inside_flatpak ());
+
app = g_desktop_app_info_new (filename);
if (parameter) {
file = g_file_new_for_path (parameter);
@@ -619,44 +624,30 @@ ephy_file_launch_desktop_file (const char *filename,
return ret;
}
-GAppInfo *
-ephy_file_launcher_get_app_info_for_file (GFile *file,
- const char *mime_type)
+static gboolean
+launch_via_uri_handler (GFile *file)
{
- GAppInfo *app = NULL;
-
- g_assert (file || mime_type);
+ const char *uri;
+ GdkDisplay *display;
+ GdkAppLaunchContext *context;
+ g_autoptr(GError) error = NULL;
- if (mime_type != NULL) {
- app = g_app_info_get_default_for_type (mime_type,
- FALSE);
- } else {
- GFileInfo *file_info;
- char *type;
-
- /* Sniff mime type and check if it's safe to open */
- file_info = g_file_query_info (file,
- G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
- 0, NULL, NULL);
- if (file_info == NULL) {
- return FALSE;
- }
- type = g_strdup (g_file_info_get_content_type (file_info));
+ display = gdk_display_get_default ();
+ context = gdk_display_get_app_launch_context (display);
- g_object_unref (file_info);
+ uri = g_file_get_uri (file);
- if (type != NULL && type[0] != '\0') {
- app = g_app_info_get_default_for_type (type, FALSE);
- }
- g_free (type);
+ g_app_info_launch_default_for_uri (uri, G_APP_LAUNCH_CONTEXT (context), &error);
+ if (error) {
+ g_warning ("Failed to launch handler for URI %s: %s", uri, error->message);
+ return FALSE;
}
- return app;
+ return TRUE;
}
/**
* ephy_file_launch_handler:
- * @mime_type: the mime type of @file or %NULL
* @file: a #GFile to pass as argument
* @user_time: user time to prevent focus stealing
*
@@ -666,25 +657,33 @@ ephy_file_launcher_get_app_info_for_file (GFile *file,
* Returns: %TRUE on success
**/
gboolean
-ephy_file_launch_handler (const char *mime_type,
- GFile *file,
- guint32 user_time)
+ephy_file_launch_handler (GFile *file,
+ guint32 user_time)
{
GAppInfo *app = NULL;
gboolean ret = FALSE;
+ g_autoptr(GList) list = NULL;
+ g_autoptr(GError) error = NULL;
g_assert (file != NULL);
- app = ephy_file_launcher_get_app_info_for_file (file, mime_type);
-
- if (app != NULL) {
- GList *list = NULL;
-
- list = g_list_append (list, file);
- ret = ephy_file_launch_application (app, list, user_time, NULL);
- g_list_free (list);
+ /* Launch via URI handler only under flatpak, because this way loses
+ * focus stealing prevention. There's no other way to open a file
+ * under flatpak, and focus stealing prevention becomes the
+ * responsibility of the portal in this case anyway. */
+ if (ephy_is_running_inside_flatpak ())
+ return launch_via_uri_handler (file);
+
+ app = g_file_query_default_handler (file, NULL, &error);
+ if (!app) {
+ g_autofree char *path = g_file_get_path (file);
+ g_warning ("No available application to open %s: %s", path, error->message);
+ return FALSE;
}
+ list = g_list_append (list, file);
+ ret = ephy_file_launch_application (app, list, user_time, NULL);
+
return ret;
}
@@ -699,6 +698,11 @@ ephy_file_open_uri_in_default_browser (const char *uri,
gboolean retval = TRUE;
GError *error = NULL;
+ /* This is impossible to implement inside flatpak. Higher layers must
+ * ensure we don't get here.
+ */
+ g_assert (!ephy_is_running_inside_flatpak ());
+
context = gdk_display_get_app_launch_context (screen ? gdk_screen_get_display (screen) :
gdk_display_get_default ());
gdk_app_launch_context_set_screen (context, screen);
gdk_app_launch_context_set_timestamp (context, timestamp);
@@ -734,7 +738,21 @@ gboolean
ephy_file_browse_to (GFile *file,
guint32 user_time)
{
- return ephy_file_launch_handler ("inode/directory", file, user_time);
+ g_autoptr(GFile) parent = NULL;
+
+ /* This is impossible to implement inside flatpak. Higher layers must
+ * ensure we don't get here.
+ */
+ g_assert (!ephy_is_running_inside_flatpak ());
+
+ parent = g_file_get_parent (file);
+
+ /* If parent is NULL, then the file is / and that would be nuts. This
+ * function is not for directories, anyway.
+ */
+ g_assert (parent);
+
+ return ephy_file_launch_handler (parent, user_time);
}
/**
@@ -968,33 +986,3 @@ ephy_open_incognito_window (const char *uri)
g_free (command);
}
-
-gboolean
-ephy_file_launch_via_uri_handler (const char *uri)
-{
- GdkDisplay *display;
- GdkAppLaunchContext *context;
- GError *error = NULL;
-
- display = gdk_display_get_default ();
- context = gdk_display_get_app_launch_context (display);
-
- g_app_info_launch_default_for_uri (uri, G_APP_LAUNCH_CONTEXT (context), &error);
-
- if (error != NULL) {
- g_warning ("Failed to launch handler for URI %s: %s", uri, error->message);
- g_error_free (error);
- return FALSE;
- }
-
- return TRUE;
-}
-
-gboolean
-ephy_file_launch_file_via_uri_handler (GFile *file)
-{
- const char *uri;
-
- uri = g_file_get_uri (file);
- return ephy_file_launch_via_uri_handler (uri);
-}
diff --git a/lib/ephy-file-helpers.h b/lib/ephy-file-helpers.h
index eff671d89..641273dba 100644
--- a/lib/ephy-file-helpers.h
+++ b/lib/ephy-file-helpers.h
@@ -71,12 +71,7 @@ gboolean ephy_file_launch_desktop_file (const char
const char *parameter,
guint32 user_time,
GtkWidget *widget);
-gboolean ephy_file_launch_application (GAppInfo *app,
- GList *files,
- guint32 user_time,
- GtkWidget *widget);
-gboolean ephy_file_launch_handler (const char *mime_type,
- GFile *file,
+gboolean ephy_file_launch_handler (GFile *file,
guint32 user_time);
gboolean ephy_file_open_uri_in_default_browser (const char *uri,
guint32 timestamp,
@@ -91,11 +86,7 @@ gboolean ephy_file_move_uri (const char
char * ephy_file_create_data_uri_for_filename (const char *filename,
const char *mime_type);
char * ephy_sanitize_filename (char *filename);
-GAppInfo * ephy_file_launcher_get_app_info_for_file (GFile *file,
- const char *mime_type);
void ephy_open_default_instance_window (void);
void ephy_open_incognito_window (const char *uri);
-gboolean ephy_file_launch_via_uri_handler (const char *uri);
-gboolean ephy_file_launch_file_via_uri_handler (GFile *file);
G_END_DECLS
diff --git a/src/prefs-dialog.c b/src/prefs-dialog.c
index 761af6d03..a2438ae11 100644
--- a/src/prefs-dialog.c
+++ b/src/prefs-dialog.c
@@ -1114,8 +1114,7 @@ css_edit_button_clicked_cb (GtkWidget *button,
if (ephy_is_running_inside_flatpak ()) {
g_file_create_async (css_file, G_FILE_CREATE_NONE, G_PRIORITY_DEFAULT, NULL, css_file_created_cb, NULL);
} else {
- ephy_file_launch_handler ("text/plain", css_file,
- gtk_get_current_event_time ());
+ ephy_file_launch_handler (css_file, gtk_get_current_event_time ());
g_object_unref (css_file);
}
}
diff --git a/src/window-commands.c b/src/window-commands.c
index 5b695294e..dd832346b 100644
--- a/src/window-commands.c
+++ b/src/window-commands.c
@@ -1872,7 +1872,7 @@ save_temp_source_close_cb (GOutputStream *ostream, GAsyncResult *result, gpointe
goto out;
}
- if (!ephy_file_launch_handler ("text/plain", file, gtk_get_current_event_time ())) {
+ if (!ephy_file_launch_handler (file, gtk_get_current_event_time ())) {
/* Fallback to view the source inside the browser */
EphyEmbed *embed;
@@ -2059,7 +2059,7 @@ window_cmd_page_source (GSimpleAction *action,
GFile *file;
file = g_file_new_for_uri (address);
- ephy_file_launch_handler ("text/plain", file, user_time);
+ ephy_file_launch_handler (file, user_time);
g_object_unref (file);
} else {
diff --git a/tests/ephy-embed-shell-test.c b/tests/ephy-embed-shell-test.c
index e305434c2..59252ea87 100644
--- a/tests/ephy-embed-shell-test.c
+++ b/tests/ephy-embed-shell-test.c
@@ -30,24 +30,6 @@
#include <glib.h>
#include <gtk/gtk.h>
-static void
-test_ephy_embed_shell_launch_handler (void)
-{
- EphyEmbedShell *embed_shell;
- gboolean ret;
- GFile *file;
-
- embed_shell = ephy_embed_shell_get_default ();
-
- file = g_file_new_for_path (TEST_DIR "/data/test.html");
- g_assert (file);
-
- ret = ephy_embed_shell_launch_handler (embed_shell, file, NULL, 0);
- g_assert (ret == FALSE);
-
- g_object_unref (file);
-}
-
static void
web_view_created_cb (EphyEmbedShell *shell, EphyWebView *view, gpointer user_data)
{
@@ -114,9 +96,6 @@ main (int argc, char *argv[])
_ephy_shell_create_instance (EPHY_EMBED_SHELL_MODE_TEST);
g_application_register (G_APPLICATION (ephy_embed_shell_get_default ()), NULL, NULL);
- g_test_add_func ("/embed/ephy-embed-shell/launch_handler",
- test_ephy_embed_shell_launch_handler);
-
g_test_add_func ("/embed/ephy-embed-shell/web-view-created",
test_ephy_embed_shell_web_view_created);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
