After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 762548 - background, online-accounts: Non-resizable dialogs have become smaller
background, online-accounts: Non-resizable dialogs have become smaller
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Background
3.19.x
Other All
: Normal normal
: ---
Assigned To: Debarshi Ray
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-23 16:34 UTC by Allan Day
Modified: 2016-03-15 12:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (2.01 MB, image/png)
2016-02-23 16:34 UTC, Allan Day
  Details
background: Ensure that the chooser is tall enough with newer GTK+ (1.20 KB, patch)
2016-03-01 13:44 UTC, Debarshi Ray
none Details | Review
background: Add a comment explaining a hack (982 bytes, patch)
2016-03-01 13:44 UTC, Debarshi Ray
rejected Details | Review
background: Ensure that the chooser is tall enough with newer GTK+ (1.72 KB, patch)
2016-03-07 18:24 UTC, Debarshi Ray
committed Details | Review
online-accounts: Fix the add account dialog's size with newer GTK+ (1.53 KB, patch)
2016-03-07 18:56 UTC, Debarshi Ray
committed Details | Review
wacom: Use the right API for setting the window width (1.29 KB, patch)
2016-03-07 19:08 UTC, Debarshi Ray
committed Details | Review
Fix the dialogs' dimensions with newer GTK+ (6.39 KB, patch)
2016-03-14 18:26 UTC, Debarshi Ray
committed Details | Review
telepathy: Ensure that the dialog is not unnecessarily tall (1.81 KB, patch)
2016-03-14 18:26 UTC, Debarshi Ray
committed Details | Review
oauth, oauth2: Stop the web view from leaking out of the dialog (1.87 KB, patch)
2016-03-14 18:27 UTC, Debarshi Ray
committed Details | Review
build: Bump minimum GTK+ version to 3.19.12 (826 bytes, patch)
2016-03-14 18:31 UTC, Debarshi Ray
committed Details | Review

Description Allan Day 2016-02-23 16:34:45 UTC
Created attachment 322004 [details]
screenshot

The choose background dialog is much wider than it is tall. This makes it look unbalanced and a bit silly. It is also awkward to use, since you can't see many backgrounds at the same time.

Can we make it taller?
Comment 1 Debarshi Ray 2016-02-23 16:40:20 UTC
(In reply to Allan Day from comment #0)
> The choose background dialog is much wider than it is tall. This makes it
> look unbalanced and a bit silly. It is also awkward to use, since you can't
> see many backgrounds at the same time.
> 
> Can we make it taller?

Sure, we can.
Comment 2 Debarshi Ray 2016-02-26 15:34:25 UTC
I am willing to argue that this is a change in gtk+'s behaviour when setting the size of a window. I have also seen this in apps like gnome-photos that try to remember the window size from the last run where the size saved via gtk-3.18.x is not respected by gtk+ 3.19.x.

In this case, the dialog used to be tall enough to show at least two full rows instead of one and a half as it is now. We can surely make it taller than two rows or use a different logic to determine the height, but would be nice to try to fix the gtk+ issue as well.

Here is how we set the size:
https://git.gnome.org/browse/gnome-control-center/tree/panels/background/cc-background-chooser-dialog.c#n110

Right now, setting a size that is 1.2 times that of the parent doesn't give a dialog that is taller than the parent.

/me goes to write a reproducer
Comment 3 Matthias Clasen 2016-02-26 17:04:56 UTC
Please pair gtk_window_get_size with gtk_window_set_default_size
Comment 4 Debarshi Ray 2016-03-01 11:34:03 UTC
(In reply to Matthias Clasen from comment #3)
> Please pair gtk_window_get_size with gtk_window_set_default_size

Indeed, that is very likely the issue here. However, Windows are taller even with paired gtk_window_get_size and gtk_window_set_default_size calls. I filed bug 762920 for that.
Comment 5 Debarshi Ray 2016-03-01 13:44:27 UTC
Created attachment 322763 [details] [review]
background: Ensure that the chooser is tall enough with newer GTK+
Comment 6 Debarshi Ray 2016-03-01 13:44:49 UTC
Created attachment 322764 [details] [review]
background: Add a comment explaining a hack
Comment 7 Debarshi Ray 2016-03-01 13:45:56 UTC
(In reply to Matthias Clasen from comment #3)
> Please pair gtk_window_get_size with gtk_window_set_default_size

Now I remember why the code doesn't use gtk_window_set_default_size: it doesn't work with non-resizable GtkWindows.
Comment 8 Bastien Nocera 2016-03-02 11:23:56 UTC
Review of attachment 322763 [details] [review]:

::: panels/background/cc-background-chooser-dialog.c
@@ +113,2 @@
       gtk_window_get_size (parent, &width, &height);
+      gtk_widget_set_size_request (GTK_WIDGET (chooser), -1, (gint) (1.1 * height));

The point of the old value was to make sure that the dialogue didn't cover the Settings window.

The commit message doesn't explain why the change is necessary though, or explain why that's needed.
Comment 9 Bastien Nocera 2016-03-02 11:24:23 UTC
Review of attachment 322764 [details] [review]:

Sure.
Comment 10 Debarshi Ray 2016-03-02 12:50:50 UTC
Let's wait for things like bug 762974 to be resolved. Olivier is also considering the option of making gtk_window_set_default_size work for non-resizable windows.
Comment 11 Debarshi Ray 2016-03-02 12:56:28 UTC
I will also note that we need similar fixes to the online accounts dialogs.
Comment 12 Debarshi Ray 2016-03-07 18:24:47 UTC
Created attachment 323308 [details] [review]
background: Ensure that the chooser is tall enough with newer GTK+
Comment 13 Debarshi Ray 2016-03-07 18:27:27 UTC
Review of attachment 322764 [details] [review]:

Thanks to ofourdan, we no longer need this hack (see the other patch). Hence, no need to document it. :)
Comment 14 Debarshi Ray 2016-03-07 18:56:50 UTC
Created attachment 323313 [details] [review]
online-accounts: Fix the add account dialog's size with newer GTK+
Comment 15 Debarshi Ray 2016-03-07 18:58:15 UTC
By the way, the commit messages are probably horrible, and the online-accounts patch needs matching changes in the gnome-online-accounts side.
Comment 16 Debarshi Ray 2016-03-07 19:08:04 UTC
Created attachment 323315 [details] [review]
wacom: Use the right API for setting the window width
Comment 17 Bastien Nocera 2016-03-08 12:22:24 UTC
Review of attachment 323308 [details] [review]:

Sure.
Comment 18 Bastien Nocera 2016-03-08 12:22:47 UTC
Review of attachment 323313 [details] [review]:

Yep.
Comment 19 Bastien Nocera 2016-03-08 12:23:08 UTC
Review of attachment 323315 [details] [review]:

Sure.
Comment 20 Bastien Nocera 2016-03-08 12:23:29 UTC
And don't forget to bump the required version of GTK+ in the configure.ac file.
Comment 21 Debarshi Ray 2016-03-09 12:52:56 UTC
(In reply to Bastien Nocera from comment #20)
> And don't forget to bump the required version of GTK+ in the configure.ac
> file.

The gtk+ version hasn't been bumped since the last release, which was before ofourdan's changes. When I spoke with him about this last week, he was a bit reluctant to bump it only for this.
Comment 22 Debarshi Ray 2016-03-09 12:54:24 UTC
Keeping it open for the corresponding gnome-online-accounts patches.
Comment 23 Debarshi Ray 2016-03-14 18:26:08 UTC
Created attachment 323902 [details] [review]
Fix the dialogs' dimensions with newer GTK+
Comment 24 Debarshi Ray 2016-03-14 18:26:37 UTC
Created attachment 323903 [details] [review]
telepathy: Ensure that the dialog is not unnecessarily tall
Comment 25 Debarshi Ray 2016-03-14 18:27:03 UTC
Created attachment 323904 [details] [review]
oauth, oauth2: Stop the web view from leaking out of the dialog
Comment 26 Debarshi Ray 2016-03-14 18:31:58 UTC
Created attachment 323905 [details] [review]
build: Bump minimum GTK+ version to 3.19.12

We now have a new enough gtk+ version that we can require.
Comment 27 Debarshi Ray 2016-03-14 18:40:20 UTC
Comment on attachment 323902 [details] [review]
Fix the dialogs' dimensions with newer GTK+

Pushed to g-o-a master.
Comment 28 Debarshi Ray 2016-03-14 18:40:32 UTC
Comment on attachment 323903 [details] [review]
telepathy: Ensure that the dialog is not unnecessarily tall

Pushed to g-o-a master.
Comment 29 Debarshi Ray 2016-03-14 18:40:43 UTC
Comment on attachment 323904 [details] [review]
oauth, oauth2: Stop the web view from leaking out of the dialog

Pushed to g-o-a master.
Comment 30 Matthias Clasen 2016-03-15 12:40:56 UTC
gtk 3.19.12 has been released, so I think you can push the last patch here and close this bug