GNOME Bugzilla – Bug 61092
yes/no questions should be WM_TRANSIENT_FOR dialogs
Last modified: 2005-01-01 21:56:06 UTC
Modal dialogs should be transient windows: For example, when the "save as" dialog asks you whether you really want to open a file, it goes modal: a yes/no question comes up, and the "save as" dialog grays out. The yes/no dialog should be marked as being a transient for the "save as" dialog, via the WM_TRANSIENT_FOR property (XSetTransientForHint). This lets window managers decorate and position the dialog correctly, and do other things like maintaining stacking order. For example, with MWM, this will cause the yes/no dialog to appear centered over the save-as dialog; make it so that when one has focus, so does the other; and make it impossible for the save-as dialog to be raised above the yes/no dialog. Not all window managers will do this sort of thing, but apps need to give them the information to make is possible.
You are right: modal dialogs should be transient windows. Many modern window managers are able to use this information correctly. In GTK+ terms, this means adding a call to gtk_window_set_transient_for () in appropriate parts of the code. But there is a problem: this function calls needs the GTK_WINDOW id of the dialog window and its parent. In app/fileops.c (gimp_1.2 branch), the function file_overwrite () does not get the id of the parent window (the save-as dialog). This could probably be fixed easily in this case because all functions dealing with the load/save dialog and the corresponding warnings and questions are in the same file. But this would be more tricky for the file plug-ins that can also generate their own warning dialogs.
Created attachment 5691 [details] [review] Patch to add WM_TRANSIENT_FOR hint to the "File Exists" dialog
The previous attachment contains a small patch that adds support for the WM_TRANSIENT_FOR hint to the "File Exists" dialog. It should be applied to the 1.2 branch (if you think that the patch is correct). I did not look at the other dialogs, but there are potentially a lot of windows that should use the WM_TRANSIENT_FOR hint. Not only the various confirmation dialogs, but also all progress bars (even if the dialog is not modal and the parent window is not insensitive).
I just realized that a better place to correct this problem could be in the all the "query_box" functions such as gimp_query_boolean_box() and others defined in libgimp/gimpquerybox.c, because all these query box should be marked as transient. However, fixing this would probably require an API change because all these functions would need the GtkWidget or GtkWindow of the appropriate parent window. So this solution could only be used in the development branch (1.4), while the previous patch is probably more appropriate for the 1.2 branch.
Sven, do you think the fix is safe to apply?
Yes, I think we should apply it and probably look through the sources to see if there are other places that need the same fix. For gimp-1.4, we should extend the API of query boxes and the like to do this automatically.
I have applied the patch to the stable tree. I will not close the bug since there are other places that need a similar fix. Those can however only addressed in gimp-1.4 due to API changes. Changed the target milestone accordingly.
Why was this changed from "minor" to "enhancement" without a word of explanation? The severity "minor" is defined as: "The component mostly works, but causes some irritation to users. A workaround should usually exist." This is precisely the case here. The Inter-Client Communication Conventions Manual says that "dialogue windows [...] should have WM_TRANSIENT_FOR set" and the GIMP is currently not following the ICCCM for some windows. So this is a bug, not just a feature that is nice to have.
Merged the patch into the HEAD branch: 2003-03-16 Sven Neumann <sven@gimp.org> * app/gui/file-save-dialog.c (file_save_overwrite): set the dialog transient to the file selection dialog (see bug #61092). I will leave the report open since there are lots of other places that need a similar fix.
2003-03-20 Sven Neumann <sven@gimp.org> * app/gui/file-commands.c * app/gui/file-new-dialog.c: made more dialogs transient for their parent window.
Changing target to 2.0 since this isn't going to block a feature freeze. Dave.
Removing the PATCH keyword since the patch that is attached here has been applied.
Fixed in CVS: 2003-11-08 Michael Natterer <mitch@gimp.org> To be multihead safe, each new window or menu needs to be associated with a GdkScreen or it will pop up on the default screen. * libgimpwidgets/gimpquerybox.[ch] * app/display/gimpdisplayshell-layer-select.[ch] * app/widgets/widgets-types.h * app/widgets/gimpitemfactory.[ch] * app/widgets/gimpitemtreeview.[ch] * app/widgets/gimptemplateview.[ch] * app/widgets/gimptooldialog.[ch] * app/widgets/gimpviewabledialog.[ch] * app/gui/channels-commands.[ch] * app/gui/color-notebook.[ch] * app/gui/convert-dialog.[ch] * app/gui/edit-commands.[ch] * app/gui/grid-dialog.[ch] * app/gui/image-commands.[ch] * app/gui/info-dialog.[ch] * app/gui/layers-commands.[ch] * app/gui/offset-dialog.[ch] * app/gui/resize-dialog.[ch] * app/gui/stroke-dialog.[ch] * app/gui/templates-commands.[ch] * app/gui/vectors-commands.[ch]: added "GtkWidget *parent" paramaters to all functions which create menus, popups or windows and pass "parent" to gimp_dialog_new() or one of the various wrappers around it. As a side effect, this fixes bug #61092. (...)