GNOME Bugzilla – Bug 771292
app window grows with each start due to incorrectly using gtk API (wayland)
Last modified: 2017-10-26 15:55:02 UTC
Description of problem: Since gtk3-3.21.5-1.fc25.x86_64 baobab window grows with each start (on wayland). You can see the same issue with virt-manager here: https://www.youtube.com/watch?v=NO6Wlaoq0x8 (the same issue applies to baobab) This has been reported against gtk: https://bugzilla.gnome.org/show_bug.cgi?id=771112 and determined as an application bug: https://bugzilla.gnome.org/show_bug.cgi?id=771112#c7 Quoting how to fix this: ~~~ https://wiki.gnome.org/HowDoI/SaveWindowState Querying the window size and then setting it with GtkWindow API is perfectly fine. Querying the window size with GtkWidget API and then setting it with GtkWindow API, *or* querying the window size with GtkWindow API and then setting it with GtkWidget API, is *not* okay, and only worked up until now with server-side decorations only on X11. ~~~ Version-Release number of selected component (if applicable): baobab-3.21.90-1.fc25.x86_64 gtk3-3.21.5-1.fc25.x86_64 How reproducible: always Steps to Reproduce: 1. start and close baobab repeatedly, it will grow each time
Created attachment 350491 [details] [review] Patch to only use GTKWindow API
Created attachment 350540 [details] [review] cleaned up code style (missing space)
Created attachment 350541 [details] [review] sorry previous one was garbage, still not used to the create patch system
Review of attachment 350541 [details] [review]: The code looks good! The commit message could use some love, it's important to explain what was happening, why, and how it's fixed. Follow our guidelines in https://wiki.gnome.org/Newcomers/SubmitPatch for commit messages (not need to use gitg though).
Review of attachment 350541 [details] [review]: ::: src/baobab-window.vala @@ +159,3 @@ configure_event.connect ((event) => { if (!(Gdk.WindowState.MAXIMIZED in get_window ().get_state ())) { + int new_width, new_height; trivial: Are "width" and "height" already used earlier in this scope? If not, I would just call the variables "width" and "height", personally, but maybe others would disagree... (Carlos said he agrees, though :)
Review of attachment 350541 [details] [review]: ::: src/baobab-window.vala @@ +159,3 @@ configure_event.connect ((event) => { if (!(Gdk.WindowState.MAXIMIZED in get_window ().get_state ())) { + int new_width, new_height; Variables named "width" and "height" are used earlier in the parent scope, so it could be confusing and possibly dangerous to reuse them (depending on how it was done)... so I withdraw my objection! Just the commit message to go :)
Created attachment 351838 [details] [review] baobab window: use Window size instead of Widget updated commit message
A similar fix was committed a few weeks ago.