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 762974 - GtkWindow: gtk_window_set_default_size() does not work with fixed size windows
GtkWindow: gtk_window_set_default_size() does not work with fixed size windows
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-03-02 10:11 UTC by Olivier Fourdan
Modified: 2016-03-05 16:12 UTC
See Also:
GNOME target: 3.20
GNOME version: ---


Attachments
Simple reproducer for testing purpose. (2.17 KB, text/plain)
2016-03-02 10:11 UTC, Olivier Fourdan
  Details
[PATCH] gtkwindow: Apply CSD to size request (1.87 KB, patch)
2016-03-02 10:16 UTC, Olivier Fourdan
rejected Details | Review
[PATCH] gtkwindow: Use default size even if not resizable (2.01 KB, patch)
2016-03-02 15:11 UTC, Olivier Fourdan
committed Details | Review
Updated reproducer using fixed size window and set_default_size() (2.23 KB, text/plain)
2016-03-02 15:16 UTC, Olivier Fourdan
  Details
[PATCH] gtkwindow: windows with a fixed size can shrink (4.69 KB, patch)
2016-03-04 15:01 UTC, Olivier Fourdan
committed Details | Review
[PATCH] gtkwindow: default size with fixed size windows (3.10 KB, patch)
2016-03-04 20:37 UTC, Olivier Fourdan
committed Details | Review

Description Olivier Fourdan 2016-03-02 10:11:16 UTC
Created attachment 322834 [details]
Simple reproducer for testing purpose.

Description:

In gtk-+3.19, GtkWindow resize API takes client side decorations into account (see bug 756618).

But GtkWidget API don't (which is fine), although a problem arise when trying to set a size request on a toplevel based on the size of another toplevel, because gtk_window_get_size() will take CSD into account whereas gtk_widget_set_size_request() won't, leading to inconsistent size depending whether CSD is used or not.

e.g.: https://git.gnome.org/browse/gnome-control-center/tree/panels/background/cc-background-chooser-dialog.c#n92

Steps to reproduce:

*** Under X11 (easier to demonstrate as it supports both CSD and SSD) ***

1. Save attached reproducer as window-size.c
2. Build with
   $ gcc -o window-size -g window-size.c $(pkg-config --cflags --libs gtk+-3.0)
3. Run the test as-is and click on the button:
   $ ./window-size
4. Redo the same with GTK_CSD=1:
   $ GTK_CSD=1 ./window-size

Actual result:

The CSD window is much smaller than that SSD one because CSD size have not been taken into account in gtk_widget_set_size_request()

Expected result:

The actual window size remains the same with both CSD and SSD.

Patch to follow.
Comment 1 Olivier Fourdan 2016-03-02 10:16:42 UTC
Created attachment 322835 [details] [review]
[PATCH] gtkwindow: Apply CSD to size request

When using the GtkWidget API gtk_widget_set_size_request() on a
GtkWindow, the actual resulting size of the window is reduced by the
client side decoration when used.

Compensate for the client side decorations for the given size when a
size request is in effect, so that the actual window does not end up
being smaller than expected.
Comment 2 Christoph Reiter (lazka) 2016-03-02 10:20:42 UTC
Wouldn't setting a size request on the child using the allocation of the other window's child work here?
Comment 3 Olivier Fourdan 2016-03-02 10:25:11 UTC
(In reply to Christoph Reiter (lazka) from comment #2)
> Wouldn't setting a size request on the child using the allocation of the
> other window's child work here?

It might (I haven't tried, should be easy though using the provided reproducer), but the problem here is we introduced an inconsistency with the CSD size changes.

Note: This is mostly an RFC patch for now, I am getting a tad lost in the numerous API to set/get size on gtk+ widgets and windows and I won't pretent I understand, even less master, the fine art of widget sizes in gtk+ - i.e. this tiny patch might wreak havoc!
Comment 4 Olivier Fourdan 2016-03-02 14:56:03 UTC
Review of attachment 322835 [details] [review]:

As discussed on irc, CSD considerations should not leak into the GtkWidget API therefore the current behaviour is to be expected so let's consider this patch as rejected.
Comment 5 Olivier Fourdan 2016-03-02 14:58:46 UTC
But then we need a way to get non-resizable CSD windows of the corect, expected size.

I reckon there is no reason not to honor the size set with gtk_window_set_default_size() on non-resizable windows.

Non-resizable applies primarily to the user, ie the developper doesn;t want the user to be able to resize the window, yet the developper should be able to set a default size and get the expected size on CSD windows even when not resizable.
Comment 6 Olivier Fourdan 2016-03-02 15:11:55 UTC
Created attachment 322873 [details] [review]
[PATCH] gtkwindow: Use default size even if not resizable

If a window is not resizable (with gtk_window_set_resizable ()),
the size given with gtk_window_set_default_size() is ignored.

The solution to this would be to use gtk_widget_set_size_request() but
that's a GtkWidget API and therefore does not take into account the
client side decorations when in use with GtkWindow.

Refactor the code so that gtk_window_set_default_size() (which is a
GtkWindow API) gives the expected result on non-resizable windows as
well.
Comment 7 Olivier Fourdan 2016-03-02 15:16:47 UTC
Created attachment 322874 [details]
Updated reproducer using fixed size window and set_default_size()
Comment 8 Emmanuele Bassi (:ebassi) 2016-03-03 12:35:33 UTC
Review of attachment 322873 [details] [review]:

For what it's worth, I do prefer this approach in terms of consistency.
Comment 9 Matthias Clasen 2016-03-03 13:22:17 UTC
Review of attachment 322873 [details] [review]:

yeah, lets do this
Comment 10 Olivier Fourdan 2016-03-03 13:50:19 UTC
Comment on attachment 322873 [details] [review]
[PATCH] gtkwindow: Use default size even if not resizable

attachment 322873 [details] [review] pushed as commit 0f95472 gtkwindow: Use default size even if not resizable
Comment 11 Matthias Clasen 2016-03-04 04:43:56 UTC
So, this causes more problems than expected. The color chooser dialog now grows when you switch to the editor, and doesn't shrink back down the next time it is presented.

I think we may have to reconsider this.
Comment 12 Matthias Clasen 2016-03-04 04:46:35 UTC
One important aspect of non-resizable windows that we need to preserve is that they shrink when their content requires less size.
Comment 13 Olivier Fourdan 2016-03-04 09:39:05 UTC
(In reply to Matthias Clasen from comment #11)
> So, this causes more problems than expected. The color chooser dialog now
> grows when you switch to the editor, and doesn't shrink back down the next
> time it is presented.

It's even worse with the expander (even though the expander has some other serious issue unrelated to this one, the text won't hide either but that was the case before this patch series).
 
(In reply to Matthias Clasen from comment #12)
> One important aspect of non-resizable windows that we need to preserve is
> that they shrink when their content requires less size.

That makes it sounds like we're relying on undocumented side effects here, none of this is actually mentioned in neither gtk_window_set_default_size() nor gtk_window_set_resizable() documentation.

We could make the default size work initially and then switch back to whatever the content requires later on, including shrinking, that should work... Although I am not sure of anything now wrt gtkwindow :)

> I think we may have to reconsider this.

You mean fixing (or trying to fix) the patch or reconsider making default size on fixed size windows at all?

If I can come up with something that doesn't break further shrinking, I still prefer the new behaviour, it's more consistent and expected.
Comment 14 Matthias Clasen 2016-03-04 11:26:03 UTC
(In reply to Olivier Fourdan from comment #13)

> (In reply to Matthias Clasen from comment #12)
> > One important aspect of non-resizable windows that we need to preserve is
> > that they shrink when their content requires less size.
> 
> That makes it sounds like we're relying on undocumented side effects here,
> none of this is actually mentioned in neither gtk_window_set_default_size()
> nor gtk_window_set_resizable() documentation.

Explicitly documented or not, this is the expected behavior of non-resizable
windows.

> We could make the default size work initially and then switch back to
> whatever the content requires later on, including shrinking, that should
> work... Although I am not sure of anything now wrt gtkwindow :)

Define "later". Do you mean the window will snap back to content size the minute anything inside changes ? I don't think that is going to work.

The current behavior would probably be ok if it only affected non-resizable windows that have an explicit default size set. The color chooser doesn't have one and is broken by this anyway.
Comment 15 Matthias Clasen 2016-03-04 13:54:31 UTC
putting this on the blocker list - if we can't fix it we need to revert it for .92
Comment 16 Olivier Fourdan 2016-03-04 15:01:10 UTC
Created attachment 323096 [details] [review]
[PATCH] gtkwindow: windows with a fixed size can shrink

One important aspect of non-resizable windows that we need to preserve
is that they shrink when their content requires less size.

Previous changes to allow the default size to be applied to fixed size
windows would have prevented all fixed size windows from shrinking when
their content requires less size.

Allow shrinking for fixed-size windows unless a default size was
specified.
Comment 17 Matthias Clasen 2016-03-04 19:19:18 UTC
Review of attachment 323096 [details] [review]:

Seems to work well now for all my examples of non-resizable windows. Thanks, well done
Comment 18 Matthias Clasen 2016-03-04 19:19:26 UTC
Review of attachment 323096 [details] [review]:

Seems to work well now for all my examples of non-resizable windows. Thanks, well done
Comment 19 Olivier Fourdan 2016-03-04 20:25:50 UTC
Now that I think of it, I got an idea, I think we can do better actually.
Comment 20 Olivier Fourdan 2016-03-04 20:37:02 UTC
Created attachment 323129 [details] [review]
[PATCH] gtkwindow: default size with fixed size windows

Allow fixed size windows with a default size to grow or shrink as the
content requires, but not smaller than the given default size.

--

Note: That obviously applies only to fixed size windows, and those without a default size would behave just like before, ie grow or shrink as their content dictates, the only difference here is that fixed size windows *with* a default size specified cannot shrink smaller than the default size...

This is probably the best I can come up with without too much intrusive and risky surgery...
Comment 21 Matthias Clasen 2016-03-04 23:53:13 UTC
Review of attachment 323129 [details] [review]:

Looks good to me.
Comment 22 Arnaud B. 2016-03-05 09:02:45 UTC
With Gtk+ master, the key-editor dialogs of dconf-editor are broken (on a Wayland session at least; only the headerbar is displayed, and the first line of text under, without background):

(dconf-editor:18407): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 584 and height -15

As setting the “resizable” property of the dialog to “true” makes it display as it should, I imagine the pushed patches are causing the problem(?)
Comment 23 Matthias Clasen 2016-03-05 15:31:31 UTC
I quickly looked at the dconf-editor key editor window. It has a default-height of 1. Setting that to -1 fixes things
Comment 24 Arnaud B. 2016-03-05 15:42:22 UTC
Stupid me, I didn’t looked at the correct file this morning. -_- Looks like it was a custom fix for a sizing problem, I’ll assume it’s not an usual thing. Sorry with that.
Comment 25 Olivier Fourdan 2016-03-05 16:04:07 UTC
Err, I haven't pushed my fix yet, closing the bug was a bit premature, I will do that now :)
Comment 26 Olivier Fourdan 2016-03-05 16:10:58 UTC
Oh, Matthias has pushed the patch already, so all good now, thanks!
Comment 27 Olivier Fourdan 2016-03-05 16:12:33 UTC
Comment on attachment 323129 [details] [review]
[PATCH] gtkwindow: default size with fixed size windows

attachment 323129 [details] [review] wa pushed as commit cdc5804 gtkwindow: default size with fixed size windows