After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 534462 - Focus issues in file chooser
Focus issues in file chooser
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
unspecified
Other All
: Normal minor
: Small fix
Assigned To: gtk-bugs
Federico Mena Quintero
: 590688 (view as bug list)
Depends on: 161489
Blocks:
 
 
Reported: 2008-05-23 09:51 UTC by rom
Modified: 2009-08-06 21:10 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
Alternative patch 1 (6.42 KB, patch)
2009-03-11 10:04 UTC, Nelson Benitez
none Details | Review
Alternative patch 2 (842 bytes, patch)
2009-03-11 10:07 UTC, Nelson Benitez
none Details | Review
Keep-the-focus-in-the-location-entry-rather-than-in-the-shortcuts-tree-view (4.07 KB, patch)
2009-05-27 20:17 UTC, Milan Bouchet-Valat
none Details | Review
Give-focus-to-the-search-entry-when-switching-to-search-mode (1.23 KB, patch)
2009-05-27 20:29 UTC, Milan Bouchet-Valat
none Details | Review
Give-focus-to-the-files-list-after-click-on-the-path-bar (879 bytes, patch)
2009-05-27 20:34 UTC, Milan Bouchet-Valat
none Details | Review
Add-focus-widget-property-to-GtkTreeView-and-use-it-in-GtkFileChooser-to-focus-location-entry (7.67 KB, patch)
2009-05-27 20:57 UTC, Milan Bouchet-Valat
none Details | Review
gtk2-bgo534462-filechooser-shortcuts-focus.diff (5.09 KB, patch)
2009-07-28 00:41 UTC, Federico Mena Quintero
committed Details | Review

Description rom 2008-05-23 09:51:34 UTC
Gnome file chooser has a very annoying problem for me.
When you save a file (in gedit, in gimp, etc...), it opens the gnome filechooser :
- on the top, the filename textfield
- on the left, the shortcuts
- on the right, the files and folders

What people want to do when saving a file is selecting a folder/shortcut, and type the filename.

For the moment, let's see what happens :
- click on a shortcut, for example "Desktop"
- try starting taping the filename directly :
  - if a shortcut begins with that letter, then it is selected
  - else a useless textfield is shown with the text typed

So if you want to select a shortcut first, then type the filename, here is what you have to do :
- click on the shortcut
- click on the filename textfield to give the focus to the filename textfield
- the default filename ("unamed") which was selected becomes unselected when you click on the textfield, so to remove it, you have to select it again
- then press "delete"
- then type the filename...

Other information:
Comment 1 Vincent Untz 2008-05-28 06:18:36 UTC
Federico might have fixed this in trunk. I don't have trunk at the moment, so reassigning to gtk+ so someone else can check.
Comment 2 Federico Mena Quintero 2008-05-28 15:24:26 UTC
Oh, nice catch.  This bug still exists; it happens because clicking on the shortcuts bar gives it the focus.

Let me see if we can figure out that the shortcuts bar was clicked rather than focused with the keyboard...
Comment 3 Nelson Benitez 2009-03-11 09:58:53 UTC
Hi, I've been working on this bug and have some conclusions and two alternative patches:

As Federico said, the problem is that when you click on the shortcuts bar (which is a gtktreeview) it gets the focus, that's normal behaviour that a widget gets focused when you click on it.

But in this case we don't want the gtktreeview shorcuts bar to take focus when we click on it, so the best approach seems to add a property to gtktreeview so that when we set that property, the gtktreeview doesn't take focus on click. That is what I did in "alternative patch 1".

Also, if we want to fix this right now in filechooser (till gtktreeview accept prior patch), we could just add a timeout to the shortcuts_button_press callback to grab the focus back to the location entry after the treeview got the focus on its button_press handler. That is done in "alternative patch 2".


Also, people who can't test patches and want a workaround, if they double click the places items instead of single click, they will get the expected behaviour.
Comment 4 Nelson Benitez 2009-03-11 10:04:25 UTC
Created attachment 130453 [details] [review]
Alternative patch 1

Adds a new property "FOCUS_ON_BUTTON_PRESS" to GtkTreeView. I'll paste here the setter:

gtk_tree_view_set_focus_on_button_press:
 * @tree_view: a #GtkTreeView
 * @enabled: %TRUE to enable @tree_view to get focused after been clicked 
 * with the mouse, %FALSE otherwise.
 *
 * Sets whether @tree_view takes focus when been single clicked with the mouse,
 * which is the default, but some applications may require this to be %FALSE 
 * to not loose focus from other widgets when the user interacts with the @tree_view
 * (e.g. Places pane of GtkFileChooser Save As dialog).
Comment 5 Nelson Benitez 2009-03-11 10:07:23 UTC
Created attachment 130456 [details] [review]
Alternative patch 2

Adds a timeout of 60 miliseconds (inappreaciated to the user) that is enough to get the focus back after the treeview got it.
Comment 6 Nelson Benitez 2009-04-26 15:04:28 UTC
ping..

I would like to hear maintainer's opinion on what alternative they like better, whether patching gtktreeview with that new property(patch 1) or just fixing it in gtkfilechooser with a timeout (patch 2).
Comment 7 Milan Bouchet-Valat 2009-05-10 13:49:47 UTC
Very good idea. But why use a timeout at all? Just giving the focus back to the entry would be enough IMHO.

Another concern is that we still have the same focus-stealing issue with the main browser view. And there, typing is useful to find a folder when many are displayed. I've a proposal to solve this: we can disable the search-as-you-type feature if the user has used the view to open another folder, and re-enable it if he only selects a folder. That would be really intuitive and would not hurt the features we now have.

That would really be nice, since I've seen many time that the need to click back in the entry is confusing base users. Then the file chooser would really rock!
Comment 8 Nelson Benitez 2009-05-10 14:39:41 UTC
(In reply to comment #7)
> Very good idea. But why use a timeout at all? Just giving the focus back to the
> entry would be enough IMHO.

I tried that, but unfortunately, in my tests, the gtktreeview takes his focus after my call to gtk_widget_grab_focus, so the need to do it in a small timeout.

> Another concern is that we still have the same focus-stealing issue with the
> main browser view. And there, typing is useful to find a folder when many are
> displayed. I've a proposal to solve this: we can disable the search-as-you-type
> feature if the user has used the view to open another folder, and re-enable it
> if he only selects a folder. That would be really intuitive and would not hurt
> the features we now have.
> 
> That would really be nice, since I've seen many time that the need to click
> back in the entry is confusing base users. Then the file chooser would really
> rock!
> 

I *think* I understand the issue you talk about (i.e. after clicking a places shortcut folder the focus don't gets back to the browser view, so the search-as-you-type works on the places shortcut, not the browser view which is what a user would expect..)

but I don't understand well your explanation about how to fix it. 

I think this would also be fixed with "alternative patch 1", as then, the gtktreeview would not get any focus, so the focus would stay on the current focused widget (browser view or location_entry, depending if we are in Open or Save mode respectively). I would need to test this as I'm talking from the top of my head.

Also it could be fixed extending the "Alternative patch 2" to detect whether the timeout should give focus to the location_entry or the browser_view.
Comment 9 Milan Bouchet-Valat 2009-05-10 14:49:55 UTC
I guess you should add a callback to the "grab-focus" signal (or the like, there are several similar signals) of the shortcut listview, instead of responding to a click. That should be the normal way of doing this.


About the second issue I raised: I was not talking about shortcuts stealing focus from the folder view, but about the folder view (not shortcuts) stealing it from the filename entry. I think the entry should always keep the focus, except if the user selects (via single click) a folder. If a folder is opened via double click, the focus should return to the entry. A special case is if the user opened a folder using the keyboard (up, down, or Enter keys): then, the focus should stay in the view since it may be needed to browse other folders. I guess this scheme should suit all the use cases we can think of. What do devs think?
Comment 10 Milan Bouchet-Valat 2009-05-27 20:17:20 UTC
Created attachment 135462 [details] [review]
Keep-the-focus-in-the-location-entry-rather-than-in-the-shortcuts-tree-view

I've investigated this problem further. Attached is a first patch that should solve the problem with the shortcuts list. I think using the "event-after" signal is cleaner than a timeout. And modifying GtkTreeView is not needed in this case, since that's basically what the patch does. I'm also setting the focus in the location entry when switching to and from other modes, else that's really strange. Comments welcome!

The second improvement I'd like to introduce is to give the focus to the location entry when you double-click on a folder in the main view. But GtkTreeView is blocking this signal, so it would have to be done in its internal code. I've the necessary patch, I'll post it soon. But I'd like to have the opinion of maintainers about that: it's adding a new property which consists in a widget we want to focus when a row is activated. I guess that can be useful for other uses.

Another trivial patch is coming, about focusing the search entry on switch. Pretty obvious.
Comment 11 Milan Bouchet-Valat 2009-05-27 20:29:44 UTC
Created attachment 135463 [details] [review]
Give-focus-to-the-search-entry-when-switching-to-search-mode

Trivial patch fixing focus in the search entry.
Comment 12 Milan Bouchet-Valat 2009-05-27 20:34:59 UTC
Created attachment 135464 [details] [review]
Give-focus-to-the-files-list-after-click-on-the-path-bar

Another trivial one, makes sense to be merged with the previous if accepted (git is nice for patchers!).
Comment 13 Milan Bouchet-Valat 2009-05-27 20:57:19 UTC
Created attachment 135466 [details] [review]
Add-focus-widget-property-to-GtkTreeView-and-use-it-in-GtkFileChooser-to-focus-location-entry

This patch returns the focus to the location entry when a row is activated using the mouse in the files list. Else (single click, or keyboard), it's kept in the tree view. This must be done from within the GtkTreeView since double click is not transmitted, and because of reasons explained below. Thus, needs to be discussed, but I think that can be useful since in many situations giving the focus to another widget is more logical when the user doesn't explicitly select (single click) a row.

In gtktreeview.c, there are the lines:
> /* If we activated the row through a double click we don't want to grab
>  * focus back, as moving focus to another widget is pretty common.
>  */
>  if (!row_double_click)
>    grab_focus_and_unset_draw_keyfocus (tree_view);
This sounds wrong to me since double click only occurs after a button release event. This means that the tree view has already been focused when we get here. I guess the focus handling has not really been tested in the file chooser.

The solution I'm using in the patch is that I give the focus back to focus_widget on double click. This is unnoticeable, and seems to best thing to do. The file chooser focus behavior is then much more predictable and efficient.
Comment 14 Nelson Benitez 2009-05-27 22:33:33 UTC
Hi Milan, just wanted to thank you!! for taking the time to look further into these issues,  you're free to mark my patches as obsolete and hope yours can get reviewed and committed in the near future..
Comment 15 Matthias Clasen 2009-05-30 04:30:48 UTC
I must say that I am not convinced that all this jerking around of the focus is an improvement. It sounds to me as if you want to make the filechooser say after each mouse operation: "type something ! still clicking ? why aren't you typing something already ?" But I haven't tried these patches yet. Maybe playing around with them will change my 
Comment 16 Nelson Benitez 2009-05-30 09:00:53 UTC
file chooser is a dialog used everywhere, so if we can avoid unnecesary clicks and make the whole "want to save/load a file" interaction quicker then I think we are improving usability and productivity for the end user.

In my opinion, these small improvements make a difference in the long run when a user says "I feel like I do more work when I'm in [x] system".
Comment 17 Milan Bouchet-Valat 2009-05-30 09:19:07 UTC
Matthias: I can understand you your POV since even when working on it I felt a little strange to return focus to a specific widget all the time. But The file chooser is a very special dialog, used for a very limited range of operations. The patches only remove (respectively) the ability to focus:
1) the shortcuts list: you don't need it to add/remove shortcuts, so that's not a problem at all.
2) the file list *using double-click*: again, the only goal of focusing this tree view is to navigate in the folders with the keyboard; to do this, simply click once on the list (what you need to do now anyway), and then the focus will be kept as long as you don't double-click.

Maybe I should lay stress on the fact that this whole focus issue with the location entry is more of a problem in the file chooser than somewhere else. Tthat's not only about the need to click in the entry to type the filename: the biggest issue is that if you do click on it, you lose the preselection of the name (vs. its extension), which leads to painfully slow operations I've been able to notice with base users (search for the entry, click it, think that there's a problem, understand, remove char by char, type). But the proof by experience may convince you! ;-)
Comment 18 Milan Bouchet-Valat 2009-06-29 19:28:43 UTC
Has anybody tested these patches to see how it feels using them? Matthias, Federico?

I'd really like us to discuss that now so that they can possibly go into 2.18 if we agree. At least, the fix about not giving the focus to the bookmarks list after a click is not controversial since you wouldn't do anything with that focus anyway. Then we can consider the file list as a separate matter.

I'm okay to look for other ways of implementing this if you prefer.
Comment 19 Federico Mena Quintero 2009-07-28 00:15:43 UTC
Thanks for investigating this, Milan :)

In the past, we've had a lot of trouble with making the shortcuts list give the focus to another widget:

Bug #148828 is about single-click in the shortcuts pane.

Bug #499940 is about not focusing the file list when a folder gets clicked in the shortcuts pane.

What I think is confusing, is that you click on a folder in the file list, then you start typing, and what you type goes to the wrong place --- to the interactive-search entry, instead of where you'd want it to be (in the location entry).

It's usually best to be as predictable as possible with focus handling.  I don't really like having an exception in the shortcuts pane; if you click on it, the focus should stay there.

I'll disable interactive search in the shortcuts pane.  I'll also make the shortcuts pane redirect unhandled keystrokes to the location entry, and focus the entry in that case.

However, to make it easy to navigate between the shortcuts pane and the file list, I'll make this bug depend on bug #161489.  That one is about making the left/right arrow keys let you switch between the shortcuts pane and the file list.
Comment 20 Federico Mena Quintero 2009-07-28 00:39:49 UTC
About the path bar... the path bar's buttons are configured to disallow focusing, so that clicking on them will leave the focus wherever it was before.  I don't think we need to do any changes here.  Or do you have a particular problem in mind?
Comment 21 Federico Mena Quintero 2009-07-28 00:41:55 UTC
Created attachment 139341 [details] [review]
gtk2-bgo534462-filechooser-shortcuts-focus.diff

Pushed as b50548e35015910f62e36b96e74a54382b97cdb2.
Comment 22 Milan Bouchet-Valat 2009-07-28 15:28:52 UTC
OK, I've not tested your patches yet, but for what you say they should fix most problems I wanted to tackle. Thanks!

Though, there's still something we need to address: that's the file list stealing focus. This was the complex part. Most of the time, the user activates a folder to enter it, and then wants to type the name of the file. But then, the file list has the focus. My complex patch Add-focus-widget-property-to-GtkTreeView-and-use-it-in-GtkFileChooser-to-focus-location-entry was trying to deal with that, by considering that you can give the focus to the file list using single-click. I'm not 100% sure that's the best solution, but I think we should consider this anyway. What do you think?
Comment 23 Federico Mena Quintero 2009-07-28 17:22:06 UTC
(In reply to comment #22)

> Though, there's still something we need to address: that's the file list
> stealing focus.

Does it actually steal focus (when?), or is it just that the focus remains in the file list when you double-click on a folder inside it?

If it's the latter, I really don't want to change the focus behavior there.  Let's try to remove "magic" focus behavior, to make things predictable.

Instead, we should focus on fixing the reasons why GtkTreeView's interactive-search is annoying, for the file chooser in particular.

In the file list, interactive search is annoying just like it was in the shortcuts pane:  because you type a filename, and a crazy little popup appears when you expect your typing to go to the location entry.  However, interactive search *is* useful in the file list, especially when you don't have the location entry enabled (e.g. in the file chooser in its default OPEN mode).

I just noticed that there is gtk_tree_view_set_search_entry(tv, entry).  The tree view will then use the entry you specify instead of popping up its own.  Ideally we should be able to just reuse the file chooser's location entry.  Do you have some time to play with this?
Comment 24 Milan Bouchet-Valat 2009-07-28 18:50:29 UTC
(In reply to comment #23)
> Does it actually steal focus (when?), or is it just that the focus remains in
> the file list when you double-click on a folder inside it?
It's not actually stealing focus. It's only that it keeps it as it should, while it's rarely useful.

> If it's the latter, I really don't want to change the focus behavior there. 
> Let's try to remove "magic" focus behavior, to make things predictable.
I share this goal, obviously.
 
> [...]
> I just noticed that there is gtk_tree_view_set_search_entry(tv, entry).  The
> tree view will then use the entry you specify instead of popping up its own. 
> Ideally we should be able to just reuse the file chooser's location entry.  Do
> you have some time to play with this?
Good idea - you're not the maintainer without reasons... ;-) I'm currently playing with this, but one of my concerns is that the typing in the location entry should preserve the extension if present, which will block the auto-completion process. Another concern can be the inconsistency between open and save modes. But let's see...


About my patches: reading them once again, I've discovered that they were trying to fix other minor issues that we should not forget.

* If you read Give-focus-to-the-files-list-after-click-on-the-path-bar, you'll see that it's removing an already present gtk_widget_grab_focus() call that had no effect because it was called too early. This can now be replaced by a key stroke redirection to the search entry in MODE_SEARCH, but we should still remove the useless function call.

* Give-focus-to-the-files-list-after-click-on-the-path-bar was dealing with *open* mode only. Then, we would like to redirect key strokes on the shortcuts list to the file list, just like we do in save mode with the location entry.
Comment 25 Milan Bouchet-Valat 2009-07-29 11:55:23 UTC
Report about my experiments with the search entry, using in gtkfilechooserdefault:

@@ -5033,6 +5033,8 @@ save_widgets_create (GtkFileChooserDefault *impl)
                    0, 0);
   gtk_widget_show (impl->location_entry);
   gtk_label_set_mnemonic_widget (GTK_LABEL (widget), impl->location_entry);
+  gtk_tree_view_set_search_entry (GTK_TREE_VIEW (impl->browse_files_tree_view), GTK_ENTRY (impl->location_entry));

Working fine when focusing the file list and choosing a file, but not the other way. If you type a letter in the entry, the first corresponding filename is set unselected in the entry. The problem is that typing something makes the tree view select it, and then our callbacks set the filename in the entry. We'd need to break that loop.

Though, I'm wondering if we really need the FileChooserEntry at all. It's displaying an list of files for autocompletion, but this is fully redundant with the tree view. The Tab key could easily be used to fill the entry with the name of the select file in the tree view, without needing the FileChooserEntry. Is there any use to it?

One problem anyway will be with the extension, as I said in my previous comment. if we only pre-select the base name, typing something will not allow the search to work. That's hard to solve given that the tree view directly accesses the entry, and does not allow us to process the text we give it. Ideas?
Comment 26 Federico Mena Quintero 2009-08-06 21:10:28 UTC
*** Bug 590688 has been marked as a duplicate of this bug. ***