GNOME Bugzilla – Bug 314721
The selection list which is given to extensions is wrong
Last modified: 2005-09-06 13:56:23 UTC
Please describe the problem: The selection list which is given to extensions is wrong when two selected files have the same MIME type. Files are missing in this list : there is at most one file in the list per MIME type. Steps to reproduce: Install nautilus-actions 0.4.alpha from http://www.grumz.net/irc/nautilus-actions-0.4.alpha.tar.gz, put nautilus-actions-0.4.alpha/config/config_gvimdiff.xml in ~/.nautilus-actions, edit it to replace /home/grumz with your home directory, put nautilus-actions-0.4.alpha/config/show_param.sh in ~/bin, restart Nautilus. (Yeah it's not very easy ; that's not my software either :-) Select two files which do not have the same MIME type. Right-click, Diff with Gvim. Do the same thing with two files that have the same MIME type. Actual results: When both files have the same MIME type it shows nothing. There is only ONE element in the list which is passed to the extension, so it can't diff. When both files don't have the same MIME type, it shows something. There was TWO elements in the list. Expected results: To work whatever the MIME types are. Does this happen every time? For sure. Other information: Tracing into nautilus isn't very funny. In fact the problem happens in src/file-manager/fm-directory-view.c, in reset_extension_actions_menu(), after get_unique_files() has been called on the selection list. get_unique_files() uses has_file_in_list() to determine if a file is already present in a list, but this function is totally broken : it compares the MIME types of files ! The solution is to compare URIs. Here is the old implementation : gboolean has_file_in_list (GList *list, NautilusFile *file) { gboolean ret = FALSE; char *mime; mime = nautilus_file_get_mime_type (file); for (; list; list = list->next) { NautilusFile *tmp_file = list->data; char *tmp_mime = nautilus_file_get_mime_type (tmp_file); if (strcmp (tmp_mime, mime) == 0) { ret = TRUE; g_free (tmp_mime); break; } g_free (tmp_mime); } g_free (mime); return ret; } And here is what I propose : static gboolean has_file_in_list (GList *list, NautilusFile *file) { gboolean ret = FALSE; char *uri; uri = nautilus_file_get_uri (file); for (; list; list = list->next) { NautilusFile *tmp_file = list->data; char *tmp_uri = nautilus_file_get_uri (tmp_file); if (strcmp (tmp_uri, uri) == 0) { ret = TRUE; g_free (tmp_uri); break; } g_free (tmp_uri); } g_free (uri); return ret; }
The problem is the same with nautilus CVS at the moment.
Created attachment 51471 [details] [review] Fix for GNOME CVS
Thanks for your bug report! This bug is indeed amazingly stupid. This get_unique_files was originally added for some obscure bonobo thingie called "MIME actions" [1]. Your solution isn't satisfying either, though: The original code had two encapsulated loops going through the selection and only adding one file per MIME type to the list. What you do is changing the inner loop to do an URI match, which will always match for the file that is currently on the plate in the outer loop. Therefore, for n selection items, you iterate 1 time through the outer and n times through the inner loop, taking a look at n! inner and n outer items, where afterwards the unique_selection list will match selection. Proposed straightforward soltuion: Remove this ugly has_file_in_list/get_unique_files heritage and just pass selection to add_extension_menu_items. Thanks for your debugging efforts! PS: Please also send your patch to the nautilus mailing list [2] for review. [1] http://cvs.gnome.org/viewcvs/nautilus/src/file-manager/fm-directory-view.c?r1=1.557&r2=1.558 [2] http://mail.gnome.org/mailman/listinfo/nautilus-list
Created attachment 51515 [details] [review] This patch implements the "straightforward" suggested solution
I'm the author of nautilus-actions and I would like to confirm that this patch fixed the problem I had on nautilus 2.10.0. Thanks again nicolas.
Yeah, that looks good to me. Should be fine to commit now. If we need any last-minute release for 2.12 we'll do a branch.
OK, I've committed a slightly modified version of attachment 51515 [details] [review]. Thanks for your efforts Nicolas!