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 720233 - Make GtkHeaderBar support all kinds of CSD content, according to the theme
Make GtkHeaderBar support all kinds of CSD content, according to the theme
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2013-12-11 07:33 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-12-13 06:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
headerbar: Remove unused private API (2.48 KB, patch)
2013-12-11 07:33 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
headerbar: Support all kinds of CSD decorations (51.99 KB, patch)
2013-12-11 07:33 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-12-11 07:33:50 UTC
This is something Matthias and I talked about today. Figured I'd write the
patches and get it out of the way...
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-12-11 07:33:52 UTC
Created attachment 263963 [details] [review]
headerbar: Remove unused private API

Don't remove gtkheaderbarprivate.h, even though it's empty,
since we'll add more to it soon.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-12-11 07:33:55 UTC
Created attachment 263964 [details] [review]
headerbar: Support all kinds of CSD decorations

Move the gtkwindow.c CSD code into GtkHeaderBar, and make it triggerable
by the show-close-button property, and remove shows-fallback-app-menu.
Comment 3 Matthias Clasen 2013-12-11 15:51:50 UTC
Review of attachment 263963 [details] [review]:

sure
Comment 4 Matthias Clasen 2013-12-11 16:01:22 UTC
Review of attachment 263964 [details] [review]:

Without looking in detail, I think I agree with most of the changes, except for the removal of the show-fallback-app-menu property - it needs to remain opt-in for application-provided headerbars. I don't mind if we turn it on by default for default csd titlebars.
Comment 5 Matthias Clasen 2013-12-11 16:07:11 UTC
or maybe that is now covered by the other show- property ?
I'll have do an actual review...
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-12-11 16:11:10 UTC
show-close-button now means "show WM decorations". So, menu/icon/minimize/maximize/close. And whatever else we add in the future.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-12-12 00:28:53 UTC
Comment on attachment 263963 [details] [review]
headerbar: Remove unused private API

Attachment 263963 [details] pushed as 37baeed - headerbar: Remove unused private API
Comment 8 Matthias Clasen 2013-12-13 02:26:37 UTC
I've revamped testtitlebar to let us test all this, and it exposed a few issues with your patch:

1. show-close-button seems to be ignored ?! I always get window decorations, regardless of the value of that property

2. changing the layout style property does not cause the headerbar to be updated - we're missing some signal handler for style changes

3. setting the layout to icon:close without having a window icon leaves a dangling separator

4. unsetting gtk-shell-shows-app-menu gives me double fallback: both the headerbar and the applicationwindow provide their fallback. My code was careful to prefer the headerbar fallback over the appwindow fallback, and never do both
Comment 9 Matthias Clasen 2013-12-13 02:30:02 UTC
5. The maximize button doesn't change its icon anymore
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-12-13 02:56:18 UTC
Fixed 1 and 5.

2 seemed to be broken already... gtk_window_style_updated doesn't seem to be called when typing into the entry... maybe some code somewhere is broken?

Can't test 3 and 4.
Comment 11 Matthias Clasen 2013-12-13 06:03:22 UTC
I've fixed 3 and 4 now.