From 042d3051c68e464d27a77e16c7258abebb04e792 Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 23 Jan 2025 17:50:05 +0100 Subject: [PATCH] app, libgimpbase: do not try to open a non existing file. Now that GimpParamSpecFile makes file validation depending on the action set in the param spec, when trying to open a non-existing file (which can happen through GUI, for instance when opening through the Document History dockable or Open Recent menu), we had a quite obscure error about a value "out of range" used "for argument 'file' (#2, type GFile)", which is because core tried to set the GFile for a non-existing file into the second parameter of the plug-in PDB. This is not a very nice error for end-users. The old error was much nicer as it used to say things like: > Could not open '/path/not/existing.png' for reading: No such file or directory But in fact, this string came from the plug-in, which means: * first that the error can be different depending on the format (inconsistency of error message); * depending on bugs in plug-ins, it may just crash or return no error messages; * finally we were making a useless call to a load plug-in while we should already know from core that it won't work. The new message is something like: > Error when getting information for file "/path/not/existing.png: No such file or directory This error is generated by core, so it will always be consistently the same, is understandable too and the plug-in won't even be called. As a side related fix, I updated the GimpParamSpecFile validation to only validate local files, just like we do in core. Remote files can always be set (and we let plug-ins handle them), at least for the time being. --- app/file/file-open.c | 43 +++++++++++++++++++++--------------- libgimpbase/gimpparamspecs.c | 2 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/app/file/file-open.c b/app/file/file-open.c index e05a8267dd..75337234b5 100644 --- a/app/file/file-open.c +++ b/app/file/file-open.c @@ -121,8 +121,7 @@ file_open_image (Gimp *gimp, } /* FIXME enable these tests for remote files again, needs testing */ - if (g_file_is_native (file) && - g_file_query_exists (file, NULL)) + if (g_file_is_native (file)) { GFileInfo *info; @@ -131,27 +130,35 @@ file_open_image (Gimp *gimp, G_FILE_ATTRIBUTE_ACCESS_CAN_READ, G_FILE_QUERY_INFO_NONE, NULL, error); - if (! info) - return NULL; - if (g_file_info_get_attribute_uint32 (info, G_FILE_ATTRIBUTE_STANDARD_TYPE) != G_FILE_TYPE_REGULAR) + if (info != NULL) { - g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, - _("Not a regular file")); + if (g_file_info_get_attribute_uint32 (info, G_FILE_ATTRIBUTE_STANDARD_TYPE) != G_FILE_TYPE_REGULAR) + { + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("Not a regular file")); + g_object_unref (info); + return NULL; + } + + if (! g_file_info_get_attribute_boolean (info, + G_FILE_ATTRIBUTE_ACCESS_CAN_READ)) + { + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("Permission denied")); + g_object_unref (info); + return NULL; + } + g_object_unref (info); + } + else + { + /* File likely does not exists. error will already have a more + * accurate reason. + */ return NULL; } - - if (! g_file_info_get_attribute_boolean (info, - G_FILE_ATTRIBUTE_ACCESS_CAN_READ)) - { - g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, - _("Permission denied")); - g_object_unref (info); - return NULL; - } - - g_object_unref (info); } if (! file_proc) diff --git a/libgimpbase/gimpparamspecs.c b/libgimpbase/gimpparamspecs.c index 4958c118e2..674ae46bc6 100644 --- a/libgimpbase/gimpparamspecs.c +++ b/libgimpbase/gimpparamspecs.c @@ -293,7 +293,7 @@ gimp_param_spec_file_validate (GParamSpec *pspec, { modifying = TRUE; } - else if (file != NULL) + else if (file != NULL && g_file_is_native (file)) { gboolean exists = g_file_query_exists (file, NULL); GFileType type = g_file_query_file_type (file, G_FILE_QUERY_INFO_NONE, NULL);