GNOME Bugzilla – Bug 750384
GtkDialog can not be moved when gtk_dialog_run from GtkListBox row-activated signal callback
Last modified: 2015-06-08 12:26:05 UTC
Hi Gtk developers, The best testcase is Gnome Control Center`s display panel, it is able to run $ gnome-control-center display in your terminal to experience it, when clicked one row of the displays GtkListBox, it will show a dialog for setting resolution and rotation, but the dialog is ***NOT*** able to move by pressing and holding the header bar. The row-activated signal callback is cc_display_panel_box_row_activated https://git.gnome.org/browse/gnome-control-center/tree/panels/display/cc-display-panel.c?h=gnome-3-16#n2354 There is show_setup_dialog https://git.gnome.org/browse/gnome-control-center/tree/panels/display/cc-display-panel.c?h=gnome-3-16#n1991 But the $ gnome-control-center background is able to move the Wallpaper dialog. And I also write some examples to test GtkDialog with GtkWindow button-press-event signal callback https://github.com/xiangzhai/leetcode/blob/master/src/gtk/hello-dlg.c#L12 and GtkComboBox changed signal callback https://github.com/xiangzhai/leetcode/blob/master/src/gtk/hello-combo.c#L74 In GtkWindow`s button-press-event signal callback, the dialog is able to move, but GtkComboBox, ***NOT*** able to move! Regards, Leslie Zhai
This is a side-effect of using gtk_dialog_run. I think this should be fixed in the control-center by not using gtk_dialog_run.
Hi Matthias, Thanks for your reply! Yes, I tested comment gtk_dialog_run https://github.com/xiangzhai/leetcode/blob/master/src/gtk/hello-combo.c#L107 It is ok now ;-) Regards, Leslie Zhai
Created attachment 304624 [details] [review] gnome-control-center display panel outputs dialog moveable As Matthias mentioned, gtk_dialog_run has a side effect, so I just removed the gtk_dialog_run, use gtk_widget_show_all instead, please pay some attention to my patch, thanks a lot!
Gnome Control Center`s display panel is able to move, and GtkComboBox popup menu`s ypos is correct ;-) https://twitter.com/xiangzhai/status/606659795404218368
And about gnome-control-center display panel`s dialog GtkComboBox popup menu issue https://bugzilla.gnome.org/show_bug.cgi?id=750372 please have a look, thanks a lot!
(In reply to Matthias Clasen from comment #1) > This is a side-effect of using gtk_dialog_run. I think this should be fixed > in the control-center by not using gtk_dialog_run. This used to work with gtk+ 3.14 though. I bisected it and came up with the following patch.
Created attachment 304665 [details] [review] main: Push the current event on the stack at the very beginning The changes in commit 13e22e20300b7312e52bba7d077fc7e231695fc1 made _gtk_window_check_handle_wm_event() indirectly depend on gtk_get_current_event_time() which relies on the current event being available on the current_events stack. Since the current event is only pushed on the stack afterwards we get an invalid timestamp which breaks ewmh window moving. This fixes the issue by pushing the current event before anything else in gtk_main_do_event() and, as a byproduct, also fixes a potential memory leak when we have a rewritten event and return early due to _gtk_window_check_handle_wm_event() being TRUE.
Carlos, can you review this ?
Comment on attachment 304665 [details] [review] main: Push the current event on the stack at the very beginning Nice catch Rui :). Note however that we replace entirely the event variable with our rewritten_event at: https://git.gnome.org/browse/gtk+/tree/gtk/gtkmain.c#n1566 Pushing the event so early will maybe get you with the received event and gtk_get_current_event() being different. I agree with the approach, but the queueing should happen after that, and the gotos above turned back into plain returns. I guess the goto label might be better "cleanup" then if we're not using it on all return paths.
(In reply to Rui Matos from comment #6) > (In reply to Matthias Clasen from comment #1) > > This is a side-effect of using gtk_dialog_run. I think this should be fixed > > in the control-center by not using gtk_dialog_run. > > This used to work with gtk+ 3.14 though. I bisected it and came up with the > following patch. yes, gtk-3-14 branch worked ;-) I will try the master branch.
(In reply to Rui Matos from comment #7) > Created attachment 304665 [details] [review] [review] > main: Push the current event on the stack at the very beginning > > The changes in commit 13e22e20300b7312e52bba7d077fc7e231695fc1 made > _gtk_window_check_handle_wm_event() indirectly depend on > gtk_get_current_event_time() which relies on the current event being > available on the current_events stack. > > Since the current event is only pushed on the stack afterwards we get > an invalid timestamp which breaks ewmh window moving. > > This fixes the issue by pushing the current event before anything else > in gtk_main_do_event() and, as a byproduct, also fixes a potential > memory leak when we have a rewritten event and return early due to > _gtk_window_check_handle_wm_event() being TRUE. cool, I have tried your patch for gtk-3-16 and master branch, it worked happily ;-)
Created attachment 304763 [details] [review] main: Push the current event on the stack before we start needing it -- (In reply to Carlos Garnacho from comment #9) > Nice catch Rui :). Note however that we replace entirely the event > variable with our rewritten_event at: > https://git.gnome.org/browse/gtk+/tree/gtk/gtkmain.c#n1566 Ups, right > Pushing the event so early will maybe get you with the received > event and gtk_get_current_event() being different. I agree with the > approach, but the queueing should happen after that, and the gotos > above turned back into plain returns. I guess the goto label might > be better "cleanup" then if we're not using it on all return paths. Sure, done
Comment on attachment 304763 [details] [review] main: Push the current event on the stack before we start needing it Now it looks good :)
6ff49ee..d87769d gtk-3-16 -> gtk-3-16 3c1a2c8..ef93257 master -> master