GNOME Bugzilla – Bug 82525
Combobox list is sticky
Last modified: 2011-02-04 16:11:52 UTC
List of recently entered addresses (opened by clicking arrow icon right of address bar) is sticky - is on top after switching desktops. I'm using Fluxbox WM, if it is important.
This looks like a gtk+ issue to me. I get the same behavior from a gtk-2 combobox and sawfish 1.0
The reason is that the popup window is override-redirect. Thus it is ignored by the wm. One solution might be to make the window managed and use the proper EWMH type to ensure that it is undecorated. If we decide to keep it override redirect for compatibility with old wms, the combo box should grab the keyboard in addition to the pointer while the popup is up, in order to prevent surprises like this from keyboard events triggering wm actions.
IMO, it should be override ridurect and grab the keyboard. There are basically two ways of doing popups: - Make them override redirect, and grab the keyboard - Make them not grab the keyboard *or* the mouse and pop down when focus is not on the widget. (Which takes care of dealing with switching screens.) A combo-box popup is much like a menu, so grabbing the keyboard make sense. On the other hand, auto-completion popups, or other popups that pop up without explicit user interaction should take the second approach. I don't see any real advantage of making the popup window managed... we need control of exactly when and where the window is mapped, so we get no benefit from the window management, just extra pain. (Grabbing the mouse/keyboard on a window managed window is a lot harder, since you have to wait for the window to be mapped, as an obvious thing.)
Created attachment 11675 [details] [review] proposed patch
The patch looks as good as far as it goes ... clearly there is a lot of duplicated code that could be cleaned up and moved into common functions, but considering that the widget is going away anyways, it's probably not worth it. The two existing problems that it would be nice to fix while doing this fix, though: - if the popup_grab_on_window() fails, it would be best to abort the popup rather than continue on with an ungrabbed popup... this probably means that the grabs need to be moved higher up in the functions. - The mixture off grabbing with GDK_CURRENT_TIME and ungrabbing with event->time that is possible here is of concern -- GDK_CURRENT_TIME may be newer than some event->time in the event queue, which would cause the ungrab to fail. I think it would be best if the code just always used gtk_get_current_event_time().
Created attachment 11770 [details] [review] new patch
Hmm, I think if the grab fails in gtk_combo_popup_button_press, you will get problems because you've already done: combo->current_button = event->button; and: gtk_button_pressed (GTK_BUTTON (button)); Those probably need to be moved inside the grab as well. Actually, for complete correctness, it's probably best to do what GtkMenu does... first grab on combo->button->window ... then, if that succeeds, go ahead, do the rest, and then repeat the grab on combo->popwin (the second grab is guaranteed to succeed since you already have the grabs yourself) Before we did that for GtkMenu, we had various problems with lost events. Might actually be easier to do it that way anyways. (Though I'm not sure how much fiddling around with this broken widget is useful :-) the really annoying bug with GtkCombo is bug 54353, which unfortunately, also a real pain to fix.)
Created attachment 12263 [details] [review] Patch as applied
It was easy enough to add the grab transferring (actually simplified the patch), so I went ahead and did that. Patch applied, HEAD only - would have required some fiddling to backport to stable. Tue Nov 12 17:10:10 2002 Owen Taylor <otaylor@redhat.com> * gtk/gtkcombo.c: Fix up grabs to be robust; grab the keyboard as well as the pointer so we won't leave the window behind if the user switches desktops with a keyboard combination. (Based on a patch from Matthias Clasen, #82525)
This bug was marked RESOLVED without a resolution, which Bugzilla does not allow (and so I am fixing it). It is assumed that the bug was intended to be marked as FIXED. If the bug should have some other resolution, please change its resolution.