GNOME Bugzilla – Bug 523071
Cannot browse other files when saving an image
Last modified: 2013-05-21 16:04:52 UTC
Please describe the problem: You can save an image from a pdf by right clicking on the image and choosing 'save as'. If you select 'By Extension' in the file chooser, you will not be able to see any files that currently exist (saving over is dangerous). Steps to reproduce: 1. open a pdf file containing an image 2. right click on the image and select 'save as' Actual results: By default the file choose has 'By Extension' selected as the file format. You'll notice that while this is selected, you are unable to view a list of current files (only folders) Expected results: Files to be visible Does this happen every time? Yes Other information:
Still reproducible with 3.8. This should be easy to fix.
Created attachment 244318 [details] [review] Make 'Save Image As' dialog more like 'Open...' This patch changes the behavior of the Save Image As dialog to be more like the Open.. dialog. The 'By Extension' filter is changed to an 'All Formats' filter that adds all supported mime-types. And the format of the added supported formats filter is changed from 'Description (extensions list)' to just 'Description'. Both the behavior and the code is now more like the 'Open..' dialog.
Created attachment 244319 [details] [review] Same patch, with better (fixed) commit message.
Review of attachment 244319 [details] [review]: Thanks!, the patch looks great, I have just a few comments. ::: shell/ev-utils.c @@ +334,3 @@ + + filter = gtk_file_filter_new (); + gtk_file_filter_set_name (filter, _("All Formats")); I think it would be useful to have an option to see all files too. All files and All formats might be a bit confusing, so we could add two new filers, "All Files" and "All Image Files" or "Supported Image Files". To create an All files filter you can add a "*" pattern to the filter. @@ +335,3 @@ + filter = gtk_file_filter_new (); + gtk_file_filter_set_name (filter, _("All Formats")); + g_slist_foreach (pixbuf_formats, (GFunc)file_filter_add_mime_types, filter); Instead of iterating the formats and mime types twice, save the all_images_filter in a variable and add the mime_types in the loop below.
Created attachment 244350 [details] [review] Changes to patch after review Thank you for your review and comments! How about this version? /Jonas
Created attachment 244351 [details] [review] Updated after review V2 Noticed that the guard against invalid formats is no longer needed in the helper function.
Im not sure about the whole g_object_set_data (G_OBJECT(filter_name), "pixbuf-format", format); I guess it for finding out the format we want to save the file in. So it's helping for the cases where a filter pointing to a specific mime is used. And when using the All Files or All Supported Files it should be NULL, and then the callback in ev-window.c will look at extension. So the latest patch is wrong and should not add "pixbuf-format" for supported filter? Or should it add NULL? Thanks again for review.
Created attachment 244352 [details] [review] Fix pixbuf-format Only add pixbuf-format data when we know the format of the selection, otherwise the extension will be the hint to the save callback.
Review of attachment 244352 [details] [review]: Looks great. I think we can also avoid iterating the mime types twice. ::: shell/ev-utils.c @@ +355,2 @@ + /* add mime types to Supported Image Files filter as well */ + file_filter_add_mime_types (format, supported_filter); Here we are getting the mime types again, iterating them and deallocating them. Just leave the loop here, or pass the images_filter to the helper function.
Created attachment 244608 [details] [review] Avoid looping the mime types twice. Something like this, maybe?
Created attachment 244609 [details] [review] Now with no strange hanging curly brackets
Comment on attachment 244609 [details] [review] Now with no strange hanging curly brackets Awesome, thanks!