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 641060 - Use proper colors for trayicons with symbolic icons
Use proper colors for trayicons with symbolic icons
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 631194 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-01-31 17:57 UTC by Dan Winship
Modified: 2011-02-07 18:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tray: add _NET_SYSTEM_TRAY_COLORS support, for symbolic icons (3.67 KB, patch)
2011-01-31 17:57 UTC, Dan Winship
needs-work Details | Review
tray: fix so that trayicons using symbolic icons get the right colors (5.50 KB, patch)
2011-01-31 17:57 UTC, Dan Winship
committed Details | Review
tray: re-sync from panel, including _NET_SYSTEM_TRAY_COLORS support (9.94 KB, patch)
2011-02-01 19:44 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2011-01-31 17:57:42 UTC
The GtkStatusIcon side of the symbolic-trayicon-colors patch got committed
long ago (although it has a few bugs, bug 640159). The gnome-panel side has
been attached to bug 614711, but not committed. This adds the patch from
there, and then sets the colors appropriately from the theme.

This, eg, makes the gpk-update-icon display white-on-transparent rather
than black-on-transparent in the mostly-black message tray. (Yes, Jon, I
know. The patch is still correct as long as we support trayicons in the
message tray at all.) It would also fix NM, if NM requested symbolic icons.

Using panel.actor to get the symbolic colors is probably wrong, and I
should probably have added "color: white" to messageTray.actor's CSS and
then used that instead. So pretend I did that.
Comment 1 Dan Winship 2011-01-31 17:57:44 UTC
Created attachment 179733 [details] [review]
tray: add _NET_SYSTEM_TRAY_COLORS support, for symbolic icons
Comment 2 Dan Winship 2011-01-31 17:57:47 UTC
Created attachment 179734 [details] [review]
tray: fix so that trayicons using symbolic icons get the right colors

The tray protocol only allows setting a single set of colors for all
symbolic trayicons. We semi-arbitrarily use panel.actor as the actor
whose colors we track.
Comment 3 Owen Taylor 2011-01-31 19:08:45 UTC
Review of attachment 179733 [details] [review]:

One minor thing

::: src/tray/na-tray-manager.c
@@ +663,3 @@
+
+  if (!manager->invisible)
+    return;

This doesn't quite make sense to me - either you need to g_return_if_fail() if na_tray_manager_set_colors() is called before na_tray_manager_manage_screen() or you need to have this and then also call 	na_tray_manager_set_colors_property() after managing a screen.
Comment 4 Owen Taylor 2011-01-31 19:13:30 UTC
Review of attachment 179734 [details] [review]:

::: src/shell-tray-manager.c
@@ +205,3 @@
+  icon_colors = st_theme_node_get_icon_colors (theme_node);
+
+  foreground.red = (icon_colors->foreground.red << 8) | icon_colors->foreground.red;

OK like this, but I think I would have just written '* 257' (or * 0x101 if you like a visual representation of duplicating hex digits)
Comment 5 Dan Winship 2011-02-01 19:44:47 UTC
Created attachment 179828 [details] [review]
tray: re-sync from panel, including _NET_SYSTEM_TRAY_COLORS support

This is a re-sync from gnome-panel after committing the 3 new patches in
bug 614711. (The parts that don't appear in any of those patches are the
random differences in gtk3 porting between panel and shell.)
Comment 6 Owen Taylor 2011-02-04 19:11:13 UTC
Review of attachment 179828 [details] [review]:

Just one problem I see

::: src/tray/na-tray-child.c
@@ +55,3 @@
       /* Set a transparent background */
+      GdkColor transparent = { 0, 0, 0, 0 }; /* only pixel=0 matters */
+      gdk_window_set_background (window, &transparent);

This doesn't work - gdk_window_set_background() doesn't pay attention to the pixel value in GdkColor any more
Comment 7 Dan Winship 2011-02-07 15:20:43 UTC
reverted that part of the re-sync patch (and attached an equivalent
anti-patch to the gnome-panel bug)

Attachment 179734 [details] pushed as bd04107 - tray: fix so that trayicons using symbolic icons get the right colors
Attachment 179828 [details] pushed as e208c7e - tray: re-sync from panel, including _NET_SYSTEM_TRAY_COLORS support
Comment 8 Owen Taylor 2011-02-07 18:59:20 UTC
*** Bug 631194 has been marked as a duplicate of this bug. ***