To nautilus developers: At the moment nautilus is unusable for changing file owners and groups, because it crashes whenever the properties window is opened. As far as I can see (or have experienced), this bug is still present in all versions from at least 3.6.3 to 3.11.2. The attached patch fixes this and related buggy behaviour when changing owner/group. Please could someone consider reviewing/applying it so we can change file owners as root again?
(ignore the current patches attached to the bugs; this one has better style compliance, fixes some errors, and is diffed against a newer version, it's just that I can't log in to replace them)
From: PBS <pbs3...@googlemail.com> Date: Fri, 22 Nov 2013 01:11:41 +0100 Subject: [PATCH] Fix bugginess and crashing with owner/group changing Prevent owner_change_callback from setting owner change fields to NULL and causing assertion failure in schedule_owner_change_timeout when they race with each other (same for changing group). This usually occurs because synch_user_menu triggers an owner change which is not recognized as trivial if the owner's fullname is different to their realname, but can also happen if the user perfectly times an owner change. Correctly find the combobox entry for the current owner, using the owner's real name. Also prevents above change from causing owner changes every 200ms for users with a non-empty 'full-name' field. Don't change the active entry in user/group combo box if changes are pending, unless it just has been refreshed in which case it is necessary, otherwise there would be no active entry. Unschedule pending owner changes if the current owner is reselected. https://bugzilla.gnome.org/show_bug.cgi?id=700492 https://bugzilla.gnome.org/show_bug.cgi?id=707987 diff --git a/src/nautilus-properties-window.c b/src/nautilus-properties-window.c --- a/src/nautilus-properties-window.c +++ b/src/nautilus-properties-window.c @@ -1371,6 +1371,12 @@ group_change_callback (NautilusFile *fil char *group; g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window)); + + if ((window->details->group_change_timeout != 0) || (window->details->group_change_file == NULL)) { + /* The callback should have been cancelled so behave as if it was. */ + return; + } + g_assert (window->details->group_change_file == file); group = window->details->group_change_group; @@ -1430,11 +1436,11 @@ schedule_group_change_timeout (NautilusP _("Cancel Group Change?"), GTK_WINDOW (window)); + window->details->group_change_timeout = 0; nautilus_file_set_group (file, group, (NautilusFileOperationCallback) group_change_callback, window); - window->details->group_change_timeout = 0; return FALSE; } @@ -1508,13 +1514,14 @@ changed_group_callback (GtkComboBox *com group = gtk_combo_box_text_get_active_text (GTK_COMBO_BOX_TEXT (combo_box)); cur_group = nautilus_file_get_group_name (file); + /* Try to change file group. If this fails, complain to user. If the group is changed + * back to its current value before the change is applied, unschedule the pending change. */ + window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); + unschedule_or_cancel_group_change (window); if (group != NULL && strcmp (group, cur_group) != 0) { - /* Try to change file group. If this fails, complain to user. */ - window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); - - unschedule_or_cancel_group_change (window); schedule_group_change (window, file, group); } + g_free (group); g_free (cur_group); } @@ -1633,6 +1640,7 @@ synch_groups_combo_box (GtkComboBox *com GList *node; GtkTreeModel *model; GtkListStore *store; + gboolean refresh_grouplist; const char *group_name; char *current_group_name; int group_index; @@ -1651,7 +1659,8 @@ synch_groups_combo_box (GtkComboBox *com store = GTK_LIST_STORE (model); g_assert (GTK_IS_LIST_STORE (model)); - if (!tree_model_entries_equal (model, 0, groups)) { + refresh_grouplist = !tree_model_entries_equal (model, 0, groups); + if (refresh_grouplist) { /* Clear the contents of ComboBox in a wacky way because there * is no function to clear all items and also no function to obtain * the number of items in a combobox. @@ -1680,7 +1689,13 @@ synch_groups_combo_box (GtkComboBox *com gtk_combo_box_text_prepend_text (GTK_COMBO_BOX_TEXT (combo_box), current_group_name); current_group_index = 0; } - gtk_combo_box_set_active (combo_box, current_group_index); + + /* Don't change the active entry if the user is scrolling because that means their changes will be unscheduled. */ + NautilusPropertiesWindow *window; + window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); + if (refresh_grouplist || ((window->details->group_change_timeout == 0) && (window->details->group_change_group == NULL))) { + gtk_combo_box_set_active (combo_box, current_group_index); + } g_free (current_group_name); g_list_free_full (groups, g_free); @@ -1786,6 +1801,12 @@ owner_change_callback (NautilusFile *fil char *owner; g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window)); + + if ((window->details->owner_change_timeout != 0) || (window->details->owner_change_file == NULL)) { + /* The callback should have been cancelled so behave as if it was. */ + return; + } + g_assert (window->details->owner_change_file == file); owner = window->details->owner_change_owner; @@ -1845,11 +1866,11 @@ schedule_owner_change_timeout (NautilusP _("Cancel Owner Change?"), GTK_WINDOW (window)); + window->details->owner_change_timeout = 0; nautilus_file_set_owner (file, owner, (NautilusFileOperationCallback) owner_change_callback, window); - window->details->owner_change_timeout = 0; return FALSE; } @@ -1930,13 +1951,14 @@ changed_owner_callback (GtkComboBox *com g_free (owner_text); cur_owner = nautilus_file_get_owner_name (file); + /* Try to change file owner. If this fails, complain to user. If the owner is changed + * back to its current value before the change is applied, unschedule the pending change. */ + window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); + unschedule_or_cancel_owner_change (window); if (strcmp (new_owner, cur_owner) != 0) { - /* Try to change file owner. If this fails, complain to user. */ - window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); - - unschedule_or_cancel_owner_change (window); schedule_owner_change (window, file, new_owner); } + g_strfreev (name_array); g_free (cur_owner); } @@ -1949,6 +1971,7 @@ synch_user_menu (GtkComboBox *combo_box, GtkTreeModel *model; GtkListStore *store; GtkTreeIter iter; + gboolean refresh_userlist; char *user_name; char *owner_name; int user_index; @@ -1969,36 +1992,42 @@ synch_user_menu (GtkComboBox *combo_box, store = GTK_LIST_STORE (model); g_assert (GTK_IS_LIST_STORE (model)); - if (!tree_model_entries_equal (model, 1, users)) { - /* Clear the contents of ComboBox in a wacky way because there - * is no function to clear all items and also no function to obtain - * the number of items in a combobox. - */ + /* Recreate the ComboBox contents, while also searching for the current owner. */ + refresh_userlist = !tree_model_entries_equal (model, 1, users); + owner_name = nautilus_file_get_owner_name (file); + owner_index = -1; + + if (refresh_userlist) { gtk_list_store_clear (store); + } - for (node = users, user_index = 0; node != NULL; node = node->next, ++user_index) { - user_name = (char *)node->data; + for (node = users, user_index = 0; node != NULL; node = node->next, ++user_index) { + user_name = (char *)node->data; - name_array = g_strsplit (user_name, "\n", 2); - if (name_array[1] != NULL) { - combo_text = g_strdup_printf ("%s - %s", name_array[0], name_array[1]); - } else { - combo_text = g_strdup (name_array[0]); - } + name_array = g_strsplit (user_name, "\n", 2); + if (name_array[1] != NULL) { + combo_text = g_strdup_printf ("%s - %s", name_array[0], name_array[1]); + } else { + combo_text = g_strdup (name_array[0]); + } + + if (strcmp(owner_name, name_array[0]) == 0) { + owner_index = user_index; + } + if (refresh_userlist) { gtk_list_store_append (store, &iter); gtk_list_store_set (store, &iter, - 0, combo_text, - 1, user_name, - -1); - - g_strfreev (name_array); - g_free (combo_text); + 0, combo_text, + 1, user_name, + -1); } - } - owner_name = nautilus_file_get_string_attribute (file, "owner"); - owner_index = tree_model_get_entry_index (model, 0, owner_name); + g_strfreev (name_array); + g_free (combo_text); + + if (owner_index != -1 && !refresh_userlist) break; + } /* If owner wasn't in list, we prepend it (with a separator). * This can happen if the owner is an id with no matching @@ -2032,7 +2061,12 @@ synch_user_menu (GtkComboBox *combo_box, g_strfreev (name_array); } - gtk_combo_box_set_active (combo_box, owner_index); + /* Don't change the active entry if the user is scrolling because that means their changes will be unscheduled. */ + NautilusPropertiesWindow *window; + window = NAUTILUS_PROPERTIES_WINDOW (gtk_widget_get_ancestor (GTK_WIDGET (combo_box), GTK_TYPE_WINDOW)); + if (refresh_userlist || ((window->details->owner_change_timeout == 0) && (window->details->owner_change_owner == NULL))) { + gtk_combo_box_set_active (combo_box, owner_index); + } g_free (owner_name); g_list_free_full (users, g_free);
-- nautilus-list mailing list nautilus-list@gnome.org https://mail.gnome.org/mailman/listinfo/nautilus-list
