GNOME Bugzilla – Bug 691040
selection is reported incorrectly in file chooser button
Last modified: 2017-08-28 04:50:00 UTC
Created attachment 232596 [details] test case to document incorrect behaviour From #674556 --------------- Federico Mena Quintero [gtk+ developer] 2012-04-23 20:42:27 UTC Fixed in commit 85c9b5c76. --------------- Fairly sure that this commit broke/changed the behaviour of the file chooser button. Small test program attached. Run program. Press "Click Me" to bring up a dialog with an FCB. Click the FCB, select "Other". Click on a folder in the left hand list, then click Open. The program will print to stdout indicating that the current folder has been changed. Then click "OK" in the dialog with the FCB. GTK+ 2.24.10: after the dialog has run, the current folder is the same as the one selected. Git 2-24: after the dialog has run, the current folder is the last one set by the program.
Got bitten by this, took a quick look at it and it is the impl->reload_state = RELOAD_EMPTY; that breaks stuff. Commenting that out and fc returns the right path.
(In reply to comment #1) Realized that maybe I should be a bit more verbose. So I post a diff. Doing this, seems to work very fine (if we even need the reload, since if the parent is getting unmapped, then what is it that needs to reload?). diff --git a/gtk/gtkfilechooserdefault.c b/gtk/gtkfilechooserdefault.c index cf8f1d9..fee96cf 100644 --- a/gtk/gtkfilechooserdefault.c +++ b/gtk/gtkfilechooserdefault.c @@ -5718,7 +5718,7 @@ toplevel_unmap_cb (GtkWidget *widget, settings_save (impl); cancel_all_operations (impl); - impl->reload_state = RELOAD_EMPTY; + impl->reload_state = RELOAD_HAS_FOLDER; } /* We monitor the focus widget on our toplevel to be able to know which widget
Hehe, ok. Bad of me, changing code without knowing what I am doing. It seems currently that the same codepath is used wether you press OK or Cancel (i.e. if the change I just proposed is used, then "Cancel" still chooses the directory you browsed to).
My bad! The root cause was that _gtk_file_chooser_settings_get_last_folder_uri() was not ensuring that the settings were actually read with ensure_settings_read(). However, it turns out that we don't use the last_folder_uri for anything meaningful anymore. That it was being used to "get the current directory when the file chooser is unmapped" is an artifact of how that code used to work. I removed all that code, and made the file chooser fall back to ->current_folder again. This fixes the selection in SELECT_FOLDER mode for GtkFileChooserButton. This is fixed in gtk-2-24 in commit cfb09e5. Apologies for the regression.
confirmed fixed.
Unfortunately this has introduced a new regression. I've attached a new example, which simply has one extra line added to set the current folder for the FC button. the effect of this call is precisely ... zero, whereas it used to work.
Created attachment 234660 [details] new example code to show new regression with this bug differs from previous example by addition of: gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER(button), "/home/paul");
*** Bug 684128 has been marked as a duplicate of this bug. ***
Umm, the new example code doesn't set the current folder. Where should it set it?
Created attachment 234987 [details] updated test case damn, sorry i uploaded a version with the line edit. here is the correct version.
!#@$%. I see what you mean. And now selection is broken when Cancel is used in the GtkFileChooserButton's internal dialog. Let me double-check GtkFileChooserButton's code. This breaks frequently, so clearly it is thinking in different terms than the underlying dialog :(
Federico, I need to do a 2.24.15 release for 3.7.5 - do you have a fix for this ?
federico: why not just back out a0280d7f and 85c9b5c76 ? the first one doesn't address any actual bug in gtk2 and the second is just a band aid to try to deal with the problems the first one created ...
I've just pushed a slew of changes to GtkFileChooserButton in gtk-2-24. This bug should be fixed as of commit 81df0059. Paul, can you please confirm this? I've resurrected the automated tests in gtk+/gtk/tests/filechooser.c - this is what let me add a test for your bug.
The test case now passes, and the functionality appears to be back in my application. The original issue (the browser window size being lost) is still fixed. I think we can consider this fixed.
Perfect, thanks for confirming. Hopefully with the tests in place the button should not break so often :)
contrary to my earlier testing, more extensive tests reveal that the file chooser and/or filechooser button is still severely broken to the point of unusability. the earlier tests failed to notice this because the test program was linked against "new" GTK, but run time execution used an existing (older) GTK install. testing against current git head reveals that the filechooser button fails to display its initial "set_current_folder()" value AND fails to respond to user interaction that changes it. more to follow.
to demo the issue(s): build GTK from gtk-2-24 build the (updated) test case below AND ensure that at run time it will use the build of GTK just created click the button marked "Click me" note that in the window that appears, the file chooser button displays no path at all. click the file chooser button to get the "menu" from the menu, select a folder watch the callbacks report that the current folder is now the one originally set (regardless of the fact that it was not displayed), and watch the selection be displayed (finally! but wait ... its not displaying what will be returned by get_current_folder()) repeat. tear hair from scalp.
<rgareus> las: set a breakpoint at gtk_file_chooser_set_current_folder_file <rgareus> las: this function is called from the combo_box_changed_cb() in gtkfilechooserbutton.c <rgareus> las: it should eventually end up in iface->set_current_folder = gtk_file_chooser_button_set_current_folder; <rgareus> las: but it does not. gtk_file_chooser_set_current_folder_file() calls itself with a couple of times with different GTK_FILE_CHOOSER_GET_IFACE() <rgareus> las: and eventually ends up in gtk_file_chooser_default_update_current_folder() <rgareus> las: querying the button's current folder however, uses the correct interface: gtk_file_chooser_button_get_current_folder() <rgareus> las: but that remains unchanged beause gtk_file_chooser_button_set_current_folder() is never called :(
Created attachment 238204 [details] [review] proposed fix this is a proposed fix for the issues seen so far. it incorporates federico's suggestion on IRC regarding what happens in combo_box_changed_cb(), and then additionally ensures that we (1) update the combo box when set_current_folder() is called AND the dialog is inactive (2) get the current file from the right place if the dialog is inactive.
(In reply to comment #20) > this is a proposed fix for the issues seen so far. > > it incorporates federico's suggestion on IRC regarding what happens in > combo_box_changed_cb(), and then additionally ensures that we (1) update the > combo box when set_current_folder() is called AND the dialog is inactive (2) > get the current file from the right place if the dialog is inactive. Than you! Your fix is necessary but not sufficient. I'm taking care of all the related stuff I can find, and have a unit test for this. Right now I'm doing a git-bisect for something that broke the tests, but it is with respect to widget sizing (!). Expect news tomorrow.
I've pushed all this to gtk-2-24 and master. Again, thanks for all the testing you've done.
I am using gtk-2-24-17 on Arch Linux and I still have blank file chooser button in Banshee. https://www.archlinux.org/packages/extra/x86_64/gtk2/
I'm afraid you'll have to wait until 2.24.18; these fixes appeared after 2.24.17 :)
It seems to be fixed in Fedora rawhide now with 2.24.18.
*** Bug 683395 has been marked as a duplicate of this bug. ***
*** Bug 531002 has been marked as a duplicate of this bug. ***
*** Bug 574514 has been marked as a duplicate of this bug. ***
*** Bug 555351 has been marked as a duplicate of this bug. ***
*** Bug 499653 has been marked as a duplicate of this bug. ***
*** Bug 355623 has been marked as a duplicate of this bug. ***
*** Bug 327243 has been marked as a duplicate of this bug. ***