GNOME Bugzilla – Bug 592435
--geometry not working
Last modified: 2010-10-10 00:34:03 UTC
Since upgrading to the fedora gnome-terminal-2.27.91-1.fc12.x86_64 package, the --geometry option not longer works. Bisected to this http://git.gnome.org/cgit/gnome-terminal/commit/?id=b2e682189b32dda2b0eca105fe1ee057e5e8c544 Avoid changing the size during realize cc Owen
*** Bug 593078 has been marked as a duplicate of this bug. ***
Just a quick note to the gnome-terminal maintainers - investigating and fixing this is not a priority for me, so it would probably be good if someone else looked into this. (About the only thing that would inspire me to look at this would be a threat to revert my patch and go back to the previous whacky way of doing things. I will be grumpy if that threat is made.)
Reverting b2e682189b32dda2b0eca105fe1ee057e5e8c544 does indeed fix the problem. "The observed effect is that depending on timing, the terminal will be reset to 80x24 when switching to a compositing window manager." So this commit makes the rather esoteric case of switching window managers work but breaks --geometry. I don't think that's a good trade; therefore I'm indeed considering reverting this commit for now.
Setting the size in a realize callback (what the code was doing before) is a really bad thing to do from a GTK+ perspective. It's completely undefined and unsupported. My hope would be that other options would be present other than "revert" and "not revert" - like debugging what's going on....
This bug is rather annoying to me -- it means I have to manually resize all my terminal windows rather than being able to specify the size from the launcher. Anyway, I tried to debug this but got lost before I got all the way to the bottom of it. Some findings: the old code called terminal_window_set_size() before gtk_window_parse_geometry(), and the new code after Owen's patch calls it after. What seems to be happening is that with the old code, gtk_window_parse_geometry() gets a chance to set the default geometry after terminal_window_set_size() calculates its default based on the default terminal screen, while in the new code, the geometry gets parsed but that default size gets overwritten. Simply removing the call to terminal_window_set_size() from where Owen's patch moved it does make --geometry start working again. Owen, I do think your attitude is a little off here -- your patch introduced a regression that looks worse than the bug it fixed (and in any case trading a new bug for an old one is really never a good trade), and you say fixing this regression is not a priority for you. I don't think it's reasonable to expect someone else to debug the problem for you, rather than simply reverting your broken patch.
Christian checked a fix into HEAD a few days ago and merged it into the gnome-2-28 branch today. http://git.gnome.org/cgit/gnome-terminal/commit/?h=gnome-2-28&id=9f8b6a3001b5c62e95367131ae9c7bc38a26c751 Looking at the code, the basic problem with geometry is that gtk_window_parse_geometry() is essentially only queueing up the new size to be applied later. If terminal_window_set_size() gets called before the window is actually shown, then that queued up size will get overwritten based on the current size of the active terminal. Christian's fix looks reasonable to me and should work - with the code flow in gnome-terminal, terminal_window_set_size() will have been called earlier, and since nothing will have changed since then, it doesn't need to be called again. But it's a little fragile against future modifications - if ordering was reworked, then things could conceivably break. I'll attach a cleanup patch against the HEAD branch that: - Backs out Christian's change - Does things a slightly more robust way and adds comments explain why
Created attachment 142166 [details] [review] Make geometry parsing more robust Instead of counting on terminal_window_set_size() not being called between gtk_window_parse_geometry(), find out the grid size that gtk_window_parse_geometry() determined and set that on the active terminal. Then if terminal_window_set_size() is called, it will get the correct new size instead of the old one. terminal_window_parse_geometry() is added to encapsulate this logic. This replaces the fix in commit 549e45a
Christian checked a fix into HEAD a few days ago and merged it into the gnome-2-28 branch today. http://git.gnome.org/cgit/gnome-terminal/commit/?h=gnome-2-28&id=9f8b6a3001b5c62e95367131ae9c7bc38a26c751 Looking at the code, the basic problem with geometry is that gtk_window_parse_geometry() is essentially only queueing up the new size to be applied later. If terminal_window_set_size() gets called before the window is actually shown, then that queued up size will get overwritten based on the current size of the active terminal. Christian's fix looks reasonable to me and should work - with the code flow in gnome-terminal, terminal_window_set_size() will have been called earlier, and since nothing will have changed since then, it doesn't need to be called again. But it's a little fragile against future modifications - if ordering was reworked, then things could conceivably break. I've attach a cleanup patch against the HEAD branch that: - Backs out Christian's change - Does things a slightly more robust way and adds comments explain why It seems to work in my testing. Probably doesn't need to be merged into the 2-28 branch.
Thanks for making the effort to fix this properly. If this bug is now fixed please mark the bug as resolved so that downstream packagers etc get notified etc.
Christian Persch, is this bug fixed or not? Was the fix part of the RC tarballs?
This seems fixed on 2-28 and trunk. I'm leaving the bug open while I consider the remaining patch (attachment 142166 [details] [review]).
Created attachment 171967 [details] [review] Make geometry parsing more robust Rebased to HEAD
Comment on attachment 171967 [details] [review] Make geometry parsing more robust Looks good to commit, just one nit: @@ -2280,24 +2280,20 @@ terminal_window_show (GtkWidget *widget) gtk_widget_get_allocation (widget, &widget_allocation); -#if 0 TerminalWindowPrivate *priv = window->priv; Need to move the |priv| declaration up (for -Wdeclaration-after-statement).
Pushed with the fix you pointed out and another small fix that showed up compiling against GTK+ 3 (patch was using widget->allocation.width rather than widget->allocation_width in the debug printf) Attachment 171967 [details] pushed as c304285 - Make geometry parsing more robust