This apparently only applies to ctrl+click and shift+click, not "rubberband" selection as baedert noted on IRC

Looks like this just fails because https://git.gnome.org/browse/gtk+/tree/gtk/gtktreeview.c#n10965 is not TRUE anymore, i.e. it works fine if you manage to keyboard-focus the treeview again after clicking one of the sidebar rows.

That's the reason, I'm not sure why the check is there or what the best way to fix the issue is.

I can't reproduce this with master. How are you changing directories, exactly ?

For example by clicking on the "Documents" bookmark. It works for me in "Recent" (which is the default) but any change makes it fail.

Created attachment 334598 [details] [review]
treeview: Accept row selections even if unfocused

When a treeview is unfocused but a row is selected and multiple rows can be selected, the next ctrl+click should select another row anyway.

I'm not sure why the treeview here (or generally?) does not get focused when clicked though.

When the tree view does not have the keyboard focus, it does not react to
key presses. The patch in comment 5 only partially fixes the bug. Clicking on
the tree view does not give it focus. Ctrl-click and Shift-click give it focus.
When it's not focused, the up and down arrows move the selection in the places
sidebar. Ctrl-a for select-all has no effect. The right arrow gives the tree
view the focus.

The documentation of gtk_tree_view_set_cursor() says:
 * This function is often followed by gtk_widget_grab_focus(tree_view) 
 * in order to give keyboard focus to the widget.

GtkFileChooserWidget contains two calls to gtk_tree_view_set_cursor() which are
not followed by gtk_widget_grab_focus(tree_view). Could that be what causes the
bug? Or some other missing gtk_widget_grab_focus(tree_view) in
GtkFileChooserWidget?

I suspect that this bug was introduced as a side-effect of the patch in
bug 767468. As a result of that patch, the tree view does not always get the
keyboard focus when it's clicked.

Created attachment 335194 [details] [review]
patch: treeview: Grab the focus if no one else has grabbed it

The patch fixes this bug without reintroducing bug 767468.

As in the patch in bug 767468, e33e23a6d9403f634003e6fc611ba7d02b5cf950, I have
used gtk_grab_get_current() to check which widget has grabbed the focus. Is that
really correct? It "queries the current grab of the default window group."
What if the tree view is inside a window that does not belong to the default
window group?

Review of attachment 335194 [details] [review]:

this looks confused to me. What gtk_grab_get_current returns has nothing to do with the focus, per se.

hmm, on the other hand, the code already looks at the grab widget

Created attachment 335293 [details] [review]
patch: treeview: Grab the focus if no one else has grabbed it

> hmm, on the other hand, the code already looks at the grab widget

Yes, a call to gtk_grab_get_current() was added in the problematic fix of
bug 767468. I doubt that it's the best way. My new patch uses
gtk_window_get_focus() instead.

Review of attachment 335293 [details] [review]:

Yes, this patch does seem to fix https://bugzilla.gnome.org/show_bug.cgi?id=770508

hmm, on the other hand, the code already looks at the grab widget(In reply to Kjell Ahlstedt from comment #7)
> I suspect that this bug was introduced as a side-effect of the patch in
> bug 767468. As a result of that patch, the tree view does not always get the
> keyboard focus when it's clicked.

I don't see it. Reverting but 767468 does not change the focus behavior when changing directories in the file chooser

I think there are two basic questions here:

- Do we want the filechooser to put focus on the file list when the sidebar selection changes ? Doing so makes keynav in the sidebar somewhat annoying. So far, the code doesn't do it

- Do we want the treeview to take focus on control- or shift-click in multi-selection mode ?

(In reply to Matthias Clasen from comment #13)
> > I suspect that this bug was introduced as a side-effect of the patch in
> > bug 767468. As a result of that patch, the tree view does not always get the
> > keyboard focus when it's clicked.
> 
> I don't see it. Reverting but 767468 does not change the focus behavior when
> changing directories in the file chooser

First don't revert anything.
Run the test case in this bug.
Select a map in the sidebar, e.g. Home.
Click somewhere in the tree view. The clicked line is selected, but the tree
view does not get the focus.
Ctrl-click another line. Nothing happens, because the sidebar still has the
keyboard focus.

Then revert the patch in bug 767468.
Run the test as before. The result is different, because the tree view gets
the focus the first time it's clicked.

(In reply to Matthias Clasen from comment #14)
> - Do we want the filechooser to put focus on the file list when the sidebar
> selection changes ? Doing so makes keynav in the sidebar somewhat annoying.
> So far, the code doesn't do it

No, I don't think so. It didn't before bug 767468 was fixed, and it does not
if my patch is applied.

> - Do we want the treeview to take focus on control- or shift-click in
> multi-selection mode ?

Not important, if you ask me. The problem with the fix of bug 767468 is that
now the tree view does not get the focus when it's clicked, whether ctrl-click,
shift-click or a simple non-modified click. And when it does not have the focus,
ctrl-click and shift-click don't work as expected. It gets the focus when the
right-arrow key is pressed, but it would be nice if it also got the focus when
it's clicked, like before.

Commit 29faa2db44b04b9cf7f4d96e32691f424490c730, which fixes bug 770508, also
fixes this file chooser bug. Simpler and better than my patches!

yay, thanks for confirming