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 642703 - layout indicator issues
layout indicator issues
Status: RESOLVED FIXED
Product: libgnomekbd
Classification: Core
Component: Indicator
unspecified
Other Linux
: Normal normal
: ---
Assigned To: libgnomekbd maintainers
Sergey V. Udaltsov
Depends on:
Blocks:
 
 
Reported: 2011-02-18 18:47 UTC by William Jon McCann
Modified: 2011-09-02 10:06 UTC
See Also:
GNOME target: 3.2
GNOME version: ---


Attachments
screenshot (16.42 KB, image/png)
2011-02-18 18:47 UTC, William Jon McCann
  Details
Use the right color for text in GkbdStatus (10.74 KB, patch)
2011-04-04 17:35 UTC, Vincent Untz
none Details | Review
patch (1.66 KB, patch)
2011-04-05 15:08 UTC, Matthias Clasen
none Details | Review

Description William Jon McCann 2011-02-18 18:47:23 UTC
Created attachment 181261 [details]
screenshot

The status icon used in the fallback session has a few issues.

1. It is using the wrong foreground color
2. It is using too much horizontal space
3. It should use a dropdown menu on click
Comment 1 Bastien Nocera 2011-02-18 19:28:43 UTC
This is all implemented in libgnomekbd.
Comment 2 Sergey V. Udaltsov 2011-02-27 20:10:35 UTC
Just checked the compat mode in jhbuild - I see no notification area (for status icons) in it. Am I missing something? No status icons appear anywhere (checked xchat, Transmission)
Comment 3 Sergey V. Udaltsov 2011-03-07 21:42:05 UTC
Lads, since I am having trouble testing it - would you mind running "jhbuild run gkbd-status-test" within gnome 2 - with with the latest libgnomekbd? What would you see in the status area? It works for me.
Comment 4 Sergey V. Udaltsov 2011-03-07 22:00:07 UTC
1. Wrong foreground color - the widget is using the foreground color from gtk theme. The parameters are "*PanelWidget*", "*PanelWidget*", GTK_TYPE_LABEL (for the state GTK_STATE_NORMAL).
What style should be used instead?
Comment 5 Frederic Peters 2011-03-17 18:00:37 UTC
Jon, do you have an answer on this?
Comment 6 William Jon McCann 2011-03-20 17:18:23 UTC
Should use the foreground color from the current style context I think.
Comment 7 William Jon McCann 2011-03-20 17:58:56 UTC
Basically the way it works is:

 * the theme colors are passed from the notification area tray to each of the status-icons through x properties
http://git.gnome.org/browse/gnome-panel/tree/applets/notification_area/main.c#n145

 * the tray icon implementation in gtk reads these
http://git.gnome.org/browse/gtk+/tree/gtk/gtktrayicon-x11.c#n546

 * and sets the colors on the image widget used by GtkStatusIcon
http://git.gnome.org/browse/gtk+/tree/gtk/gtkstatusicon.c#n1673

So, I think your trick is to grab the image widget out of GtkStatusIcon and read the colors set on its style context.

The image is added to the tray_icon:
gtk_container_add (GTK_CONTAINER (priv->tray_icon), priv->image);

But neither of them are readily available in the public GtkStatusIcon API.

Hmm.
Comment 8 William Jon McCann 2011-03-20 18:17:48 UTC
The reason it needs the foreground color is because gkbd-status.c renders a pixbuf itself and passes that to the status icon.

So, I think we either need to find a way for gkbd-status.c to get the color information, or gkbd-status.c needs to send the image to GtkStatusIcon using symbolic colors (like symbolic icons do).
Comment 9 William Jon McCann 2011-03-20 19:05:04 UTC
Asked for a quick opinion from Owen on IRC:

<owen> I mean, my instinct is:
<owen> cut and paste the front of the priv structure into the gnomekbd code - you just need the first member
<owen> tray_icon
<owen> and you just need to know it is a gobject
<owen> then you can g_object_get() the fg-color property
<owen> But obviously, this would need a gtk+ bug report and bug reference in a comment to be vaguely acceptable
<mccann> and get notify
<owen> Yeah,. notify will be there for free because it's a property, and it's likely not set until the icon is embedded, so you probably need it
Comment 10 Benjamin Otte (Company) 2011-03-20 19:15:05 UTC
Hrm, this is some usage that the API wasn't meant to handle. And there's no API inside GTK to achieve this goal. So the only hack that would work without API additions to GTK would be to use the window from gtk_status_icon_get_x11_window_id() to:
1) ... query the X properties manually
2) ... g_object_get_property (gdk_window_get_user_data (window), "error-color", &error_color, NULL);

But both of those are very ugly hacks...

I do think you're supposed to write a real applet if you want to display anything but static images instead of using a notification area.
Comment 11 Sergey V. Udaltsov 2011-03-20 20:05:28 UTC
Static images... do you mean ... hehe, flags?:)

There was a real indicator till 2.28 or smth. Perhaps I should recreate it. But doing that just for fallback mode would be funny...
Comment 12 Vincent Untz 2011-04-04 17:35:54 UTC
Created attachment 185125 [details] [review]
Use the right color for text in GkbdStatus

Raw first version of the approach suggested by Owen in comment 9. Seems to work.
Comment 13 Vincent Untz 2011-04-04 17:39:23 UTC
FWIW, if anybody is afraid of the mix between the global configuration and the per-status icon configuration, I'll simply point out that it's already broken in gkbd_status_size_changed(). I think the code should just be reworked to remove the global configuration anyway.
Comment 14 Sergey V. Udaltsov 2011-04-04 20:03:52 UTC
Well, the hack with private tray_icon looks ... like a hack - but since we do not have anything better than that, let's use it. I hope you do not mind if I commit it after 3.0.

Sorry, did not get the question about the mix of two configurations.
Comment 15 William Jon McCann 2011-04-04 22:14:37 UTC
svu: any idea about the size issue?
Comment 16 Matthias Clasen 2011-04-05 15:08:29 UTC
Created attachment 185209 [details] [review]
patch
Comment 17 Matthias Clasen 2011-04-05 15:16:08 UTC
This patch works around the worst of the size problems. 
There's a bunch of bugs both gtk-side and panel-side...
Comment 18 Sergey V. Udaltsov 2011-04-05 18:45:13 UTC
Matthias, thanks. They look more like hacks than fixes - but I understand they are workarounds.. So I'll commit.
Comment 19 Sergey V. Udaltsov 2011-04-05 20:16:51 UTC
Since 3.0 is out, I committed both patches. Since r-t did not express explicit excitement about 3.0.0.1 - they will be included into 3.0.1.

I am not closing this bug till we have proper, non-hackish fix for the colors (the blocking bug should be closed first)
Comment 20 Vincent Untz 2011-04-06 04:29:11 UTC
(In reply to comment #19)
> Since 3.0 is out, I committed both patches. Since r-t did not express explicit
> excitement about 3.0.0.1 - they will be included into 3.0.1.

We actually want 3.0.0.1 :-) It's just that you asked me while I was away. I'll roll a tarball right now. Thanks for your help, btw! This improves the fallback mode quite a bit.
Comment 21 Vincent Untz 2011-04-06 09:43:35 UTC
Moving target to 3.2, now that we've landed the hacky fixes for 3.0.
Comment 22 Sergey V. Udaltsov 2011-09-01 17:37:28 UTC
Committed some while ago, sorry for keeping the bug open