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 731187 - Use csd shadows for menus
Use csd shadows for menus
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 731186 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-06-04 01:00 UTC by Matthias Clasen
Modified: 2014-06-10 13:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (4.54 KB, patch)
2014-06-04 01:00 UTC, Matthias Clasen
none Details | Review
Add a csd style class (1.32 KB, patch)
2014-06-07 02:09 UTC, Matthias Clasen
committed Details | Review
Allow csd for override-redirect windows (3.48 KB, patch)
2014-06-07 02:10 UTC, Matthias Clasen
committed Details | Review
GtkMenu: take csd shadows into account for placement (1.71 KB, patch)
2014-06-07 02:10 UTC, Matthias Clasen
committed Details | Review
GtkMenu: Add the menu style class to the toplevel (953 bytes, patch)
2014-06-07 02:11 UTC, Matthias Clasen
reviewed Details | Review
Request csd for menus (843 bytes, patch)
2014-06-07 02:11 UTC, Matthias Clasen
committed Details | Review
GtkTooltip: remove hardcoded tooltip drawing (5.53 KB, patch)
2014-06-07 02:12 UTC, Matthias Clasen
committed Details | Review
GtkTooltip: take csd shadows into account for placement (1.64 KB, patch)
2014-06-07 02:13 UTC, Matthias Clasen
committed Details | Review
Request csd for tooltips (818 bytes, patch)
2014-06-07 02:13 UTC, Matthias Clasen
committed Details | Review
Add some theming for csd popups (1.06 KB, patch)
2014-06-07 02:13 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2014-06-04 01:00:48 UTC
Created attachment 277853 [details] [review]
patch

Something like this is needed to get rid of the adwaita theme engine, which currently just contains code to set up fallbacks for menu borders under non-compositing wms.
Comment 1 Matthias Clasen 2014-06-04 01:12:52 UTC
Prior art for this is in bug 660271
Comment 2 Cosimo Cecchi 2014-06-04 06:32:39 UTC
Review of attachment 277853 [details] [review]:

The idea looks good to me, but I'm worried that this might cause problems for POPUP windows that are not menus, like tooltips.

::: gtk/gtkwindow.c
@@ +1619,3 @@
       priv->type = g_value_get_enum (value);
+      if (priv->type == GTK_WINDOW_POPUP)
+        gtk_style_context_add_class (gtk_widget_get_style_context (GTK_WIDGET (window)), "popup");

I wonder if this wouldn't be better in GtkMenu itself instead - e.g. tooltips and others also use GTK_WINDOW_POPUP.

@@ +5571,2 @@
   if (priv->type == GTK_WINDOW_POPUP)
+    return TRUE;

This will also enable CSD for tooltips and other things though...
Comment 3 Matthias Clasen 2014-06-04 09:58:05 UTC
I should mention that this has been only lightly tested, with metacity/gnome-shell and menus/comboboxes.

Good point about tooltips, I'll think about that
Comment 4 André Klapper 2014-06-04 15:47:29 UTC
*** Bug 731186 has been marked as a duplicate of this bug. ***
Comment 5 Matthias Clasen 2014-06-07 02:09:28 UTC
Created attachment 278068 [details] [review]
Add a csd style class
Comment 6 Matthias Clasen 2014-06-07 02:10:01 UTC
Created attachment 278069 [details] [review]
Allow csd for override-redirect windows
Comment 7 Matthias Clasen 2014-06-07 02:10:44 UTC
Created attachment 278070 [details] [review]
GtkMenu: take csd shadows into account for placement
Comment 8 Matthias Clasen 2014-06-07 02:11:18 UTC
Created attachment 278071 [details] [review]
GtkMenu: Add the menu style class to the toplevel
Comment 9 Matthias Clasen 2014-06-07 02:11:48 UTC
Created attachment 278072 [details] [review]
Request csd for menus
Comment 10 Matthias Clasen 2014-06-07 02:12:25 UTC
Created attachment 278073 [details] [review]
GtkTooltip: remove hardcoded tooltip drawing
Comment 11 Matthias Clasen 2014-06-07 02:13:00 UTC
Created attachment 278074 [details] [review]
GtkTooltip: take csd shadows into account for placement
Comment 12 Matthias Clasen 2014-06-07 02:13:25 UTC
Created attachment 278075 [details] [review]
Request csd for tooltips
Comment 13 Matthias Clasen 2014-06-07 02:13:53 UTC
Created attachment 278076 [details] [review]
Add some theming for csd popups
Comment 14 Matthias Clasen 2014-06-09 12:01:03 UTC
Review of attachment 278076 [details] [review]:

::: gtk/resources/theme/gtk-default.css
@@ +864,3 @@
+
+.window-frame.menu {
+  border-color: darker (@bg_color);

Lapo pointed out that the space after darker needs to go
Comment 15 Matthias Clasen 2014-06-09 12:01:16 UTC
Review of attachment 278076 [details] [review]:

.
Comment 16 Cosimo Cecchi 2014-06-09 15:11:52 UTC
Review of attachment 278068 [details] [review]:

Looks good
Comment 17 Cosimo Cecchi 2014-06-09 15:13:17 UTC
Review of attachment 278069 [details] [review]:

Looks good
Comment 18 Cosimo Cecchi 2014-06-09 15:13:41 UTC
Review of attachment 278070 [details] [review]:

OK
Comment 19 Cosimo Cecchi 2014-06-09 15:20:13 UTC
Review of attachment 278071 [details] [review]:

Not a big fan of the double style class on parent and child, I can see how that could become a bit confusing. Perhaps you can just use a different one for now, and then later clean up to see if we can get away with just setting it on the toplevel?
Comment 20 Cosimo Cecchi 2014-06-09 15:20:26 UTC
Review of attachment 278072 [details] [review]:

OK
Comment 21 Cosimo Cecchi 2014-06-09 15:22:02 UTC
Review of attachment 278073 [details] [review]:

I'm all for this, but tooltips will stop being rounded under Metacity/non-composited WMs with this patch.
Comment 22 Cosimo Cecchi 2014-06-09 15:22:22 UTC
Review of attachment 278074 [details] [review]:

OK
Comment 23 Cosimo Cecchi 2014-06-09 15:22:36 UTC
Review of attachment 278075 [details] [review]:

OK
Comment 24 Matthias Clasen 2014-06-09 22:40:15 UTC
Attachment 278068 [details] pushed as 60cd707 - Add a csd style class
Attachment 278069 [details] pushed as bde4e86 - Allow csd for override-redirect windows
Attachment 278070 [details] pushed as e9ed210 - GtkMenu: take csd shadows into account for placement
Attachment 278072 [details] pushed as dae252e - Request csd for menus
Attachment 278073 [details] pushed as 49bb39e - GtkTooltip: remove hardcoded tooltip drawing
Attachment 278074 [details] pushed as 179d6a4 - GtkTooltip: take csd shadows into account for placement
Attachment 278075 [details] pushed as a39985a - Request csd for tooltips
Attachment 278076 [details] pushed as fbc6a0c - Add some theming for csd popups