From 636b38be4fefbe245305508e846cd8a075d1dcd3 Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 26 Oct 2023 17:06:27 +0200 Subject: [PATCH] app: fix broken multi-selection. This was broken in commit 3e101922. Setting a cursor basically resets to a single selection, invalidating pointer-made multi-selection. But then we got back the bug it fixes, which is that we must grab focus after the selection is actually made. So we now grab at the end. This also had a bad consequence for multi-selection (again): if the focus was not already on the tree view, gimp_container_tree_view_selection_changed() was not called. This function was where the actual selection-changing is meant to happen. So we had to shift-click (or ctrl-click) twice. The first time, nothing would happen (but focus was given to the tree view). The second time, we could finally update the selection. This is why we add 2 different cases of focus grab, which should hopefully handle all cases correctly, though this code is really extra-complex. This replaces MR !1128. --- app/widgets/gimpcontainertreeview.c | 32 ++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/app/widgets/gimpcontainertreeview.c b/app/widgets/gimpcontainertreeview.c index 015da1f78f..ab5b5fb07c 100644 --- a/app/widgets/gimpcontainertreeview.c +++ b/app/widgets/gimpcontainertreeview.c @@ -1265,6 +1265,7 @@ gimp_container_tree_view_button (GtkWidget *widget, GimpContainerView *container_view = GIMP_CONTAINER_VIEW (tree_view); GtkTreeViewColumn *column; GtkTreePath *path; + gboolean handled = TRUE; tree_view->priv->dnd_renderer = NULL; @@ -1278,17 +1279,10 @@ gimp_container_tree_view_button (GtkWidget *widget, GtkCellRenderer *edit_cell = NULL; GdkRectangle column_area; GtkTreeIter iter; - gboolean handled = TRUE; gboolean multisel_mode; GdkModifierType modifiers = (bevent->state & gimp_get_all_modifiers_mask ()); - /* Confirm the path is set before grabbing focus, as it can cause - * the list to auto-scroll to the top on first click otherwise - */ - gtk_tree_view_set_cursor (GTK_TREE_VIEW (widget), path, NULL, FALSE); - if (bevent->type != GDK_BUTTON_RELEASE && ! gtk_widget_has_focus (widget)) - gtk_widget_grab_focus (widget); - + handled = TRUE; multisel_mode = (gtk_tree_selection_get_mode (tree_view->priv->selection) == GTK_SELECTION_MULTIPLE); @@ -1303,6 +1297,19 @@ gimp_container_tree_view_button (GtkWidget *widget, multisel_mode = FALSE; } + /* We need to grab focus after a button click, in order to make keyboard + * navigation possible. For multi-selection though, actual selection will + * happen in gimp_container_tree_view_selection_changed() but the widget + * must already be focused or the handler won't run. + * Whereas for single selection, grab must happen after we changed the + * selection (which will happen in this function) otherwise we end up + * first scrolling to the current selection. So we have a separate + * gtk_widget_grab_focus() at the end of the function. + * See also commit 3e101922 and MR !1128. + */ + if (multisel_mode && bevent->type == GDK_BUTTON_PRESS && ! gtk_widget_has_focus (widget)) + gtk_widget_grab_focus (widget); + gtk_tree_model_get_iter (tree_view->model, &iter, path); renderer = gimp_container_tree_store_get_renderer (GIMP_CONTAINER_TREE_STORE (tree_view->model), &iter); @@ -1609,7 +1616,7 @@ gimp_container_tree_view_button (GtkWidget *widget, gtk_tree_path_free (path); g_object_unref (renderer); - return multisel_mode ? handled : (bevent->type == GDK_BUTTON_RELEASE ? FALSE : TRUE); + handled = (multisel_mode ? handled : (bevent->type == GDK_BUTTON_RELEASE ? FALSE : TRUE)); } else { @@ -1618,8 +1625,13 @@ gimp_container_tree_view_button (GtkWidget *widget, gimp_editor_popup_menu_at_pointer (GIMP_EDITOR (tree_view), (GdkEvent *) bevent); } - return TRUE; + handled = TRUE; } + + if (handled && bevent->type == GDK_BUTTON_PRESS && ! gtk_widget_has_focus (widget)) + gtk_widget_grab_focus (widget); + + return handled; } /* We want to zoom on each 1/4 scroll events to roughly match zooming