GNOME Bugzilla – Bug 420285
GtkFileChooserDialog resizes indefinetely in matchbox
Last modified: 2007-05-18 15:13:02 UTC
When you open a Save GtkFileChooserDialog in matchbox or without a window manager and expand it, it will enter in a cycle of resizes that only stops after collapsing it again. The dialog receives configure events with the widths 564, 384, 564, 384, 564,... until you collapse again. 384 is the requisition width and 564 is the min_width set in file_chooser_widget_update_hints(). gtk_window_compute_hints() will return 384 as max_width and 564 as min_width (shouldn't we have an assert here?). This is because *update_hints() sets max_width to -1, so *compute_hints() will set it to requisition.width. We have now fixed this internally in file_chooser_widget_update_hints(): - geometry.max_width = (priv->resize_horizontally?G_MAXSHORT:-1); + geometry.max_width = (priv->resize_horizontally?G_MAXSHORT:geometry.min_width); But this perhaps is not the right fix, and perhaps a more serious problem lies in gtk_window_compute_hints(). What do you think? Makes sense this explanation?
Looks like http://bugs.debian.org/420021 It happens for several other window managers (but not metacity nor kwm).
In Gentoo there are reports of this problem in fluxbox and WindowMaker too. At least in one case (fluxbox system, I believe) the loop starts on expander collapse, not expand.
Created attachment 87798 [details] [review] patch This patch does the following changes to the GtkFileChooserDialog and internals sizing: - moves default size and resizable state management to GtkFileChooserDefault, makes some sense to me, as it's the widget which really implements GtkFileChooserEmbed, how it is now it feels like the whole implementation is divided between those. - changes GtkFileChooserEmbed to just return one boolean value instead of resize_horizontally and resize_vertically - simplify size request in the dialog, remove the realized/unrealized sizing paths. - avoid using gtk_window_set_geometry_hints, dealing with window managers that way feels delicate. - make sure that the settings are loaded then the filechooser action changes, this way the save expander state will be set before the window is mapped I think this patch also fixes bugs #424299 and #424309
(In reply to comment #3) > > I think this patch also fixes bugs #424299 and #424309 It does, at least for me, thanks.
Created attachment 87812 [details] [review] gtk+ 2.10.12 patch The edge I live on is probably not bleeding enough... The original patch does not apply cleanly to 2.10.12, so I fixed and rediffed it for this version -- and I'm attaching the result. I hope this bug will get fixed in 2.10.
Created attachment 88149 [details] [review] new patch, applies cleanly to trunk
Confirming that the trunk patch fixes the flickering for sawfish.
I'd like to see federico's take on this, since he did a lot of the filechooser sizing code, IIRC
The patch against 2.10.12 gives a filechooser which is maximized in height. Vanilla 2.10.12 doesn't have this problem.
(In reply to comment #9) > The patch against 2.10.12 gives a filechooser which is maximized in height. > Vanilla 2.10.12 doesn't have this problem. * What is your screen resoulution? * What is your /desktop/gnome/font_rendering/dpi from GConf? Other than that, the patch looks really good to me --- and Carlos is, again, my hero. The patch works fine for me under Metacity; I didn't test it under legacy^H^H^H^Hother window managers :)
It's 1280x800 on a 12" screen. Should be 124dpi, but I configured it as 96dpi using the font preferences. 2.10.12 was already a bit bigger than usual, but that one was acceptable. With the patch, it's maximized in vertical resolution (filechooser dialog goes from top panel to bottom panel). I have one report from a user who uses xfce on a 1024x768 screen: http://bugs.archlinux.org/task/7154 screenshot is attached, same as in my case: maximized in height.
I see. We should limit the requested size to about 75% of the screen's dimensions. Carlos, feel free to commit your patch if you do something like this: impl->default_width = MAX (font_size * NUM_CHARS, gdk_screen_get_width (screen) * 0.75); impl->default_height = MAX (font_size * NUM_LINES, gdk_screen_get_height (screen) * 0.75);
That was supposed to be done already by clamp_to_screen() in gtkfilechooserdialog.c, but it seems it could not be called when the widget is realized under some circumstances, I've forced it to end up calling that function when the dialog is being mapped, that should guarantee that the dialog is clamped if it exceedes that size. I've committed already to trunk, thanks a lot :) 2007-05-18 Carlos Garnacho <carlos@imendio.com> Refactor GtkFileChooserDialog sizing. * gtkfilechooserembed.[ch] (delegate_get_resizable_hints) (_gtk_file_chooser_embed_get_resizable_hints): s/resizable_hints/resizable/, return just one boolean value to determine whether the filechooser should be resizable or not. * gtkfilechooserprivate.h (struct GtkFileChooserDialogPrivate): remove variables related to the GtkFileChooserEmbed get_default_size() and get_resizable() implementations. (struct GtkFileChooserDefault): Move default size management here. * gtkfilechooserdefault.c (gtk_file_chooser_default_size_allocate): Added, store currently allocated size to calculate default size later. (gtk_file_chooser_default_get_resizable_hints): s/resizable_hints/resizable/. (gtk_file_chooser_default_set_property): Reload settings if the file chooser action changes, this way the save expander state will be known before mapping the window, avoiding wrong window positioning and flickering. (#424299, #424309) (find_good_size_from_style): Only get size from style if it wasn't set previously. (gtk_file_chooser_default_get_default_size): return default size based on stored default size and preview/extra widget sizes. * gtkfilechooserdialog.c (file_chooser_widget_update_hints) (file_chooser_widget_realized_size_changed) (file_chooser_widget_unrealized_size_changed): simplified to (file_chooser_widget_size_changed): set window size and resizability based on the GtkFileChooserEmbed interface implementation. (Bug #420285, Tomeu Vizoso) (gtk_file_chooser_dialog_map): force a dialog size change, so it's clamped for sure to the 75% of the screen size.
Awesome, thanks, Garnacho!