This is an accessibility bug. The new settings manager main window does not show the focus widget, not it display the selected item and there is no keyboard navigation.
The reason is cuz the icon view is set to GTK_SELECTION_NONE... which was a workaround because ExoIconView is broken theme-wise and sometimes has the same color text as selection color in some themes. Go ahead and fix the selection mode, but while you're at it, find the bug relating to the theme issues and reopen it... and finding a fix would be nice too ^_^.
(In reply to comment #1) > The reason is cuz the icon view is set to GTK_SELECTION_NONE... which was a > workaround because ExoIconView is broken theme-wise and sometimes has the same > color text as selection color in some themes. Go ahead and fix the selection > mode, but while you're at it, find the bug relating to the theme issues and > reopen it... and finding a fix would be nice too ^_^. I asked Benny about this and he said that GtkCellRendererText worked fine with ExoIconView. ExoIconView passes the flags GTK_CELL_RENDERER_SELECTED, GTK_CELL_RENDERER_PRELIT and GTK_CELL_RENDERER_INSENSITIVE to gtk_cell_renderer_render() depending on the state of each item. So IMHO ExoIconView is not broken. Isn't it the cell renderer's job to take care of the rendering of the background of a cell? It looks like this is not done properly in GTK+ either. From the source of GtkIconView (GTK+ 2.14.4): 3027 /* FIXME we hardwire background drawing behind text 3028 * cell renderers here 3029 */ 3030 if (GTK_IS_CELL_RENDERER_TEXT (info->cell)) 3031 { 3032 gdk_cairo_set_source_color (cr, >K_WIDGET (icon_view)->style->base[state]); 3033 cairo_rectangle (cr, 3034 x - item->x + box.x, 3035 y - item->y + box.y, 3036 box.width, box.height); 3037 cairo_fill (cr); 3038 } So, how to proceed from here? Forget about accesibility for 4.6 or add the hack above to ExoIconView? Either way I think we should bug the GTK+ guys.
(In reply to comment #2) > (In reply to comment #1) > > The reason is cuz the icon view is set to GTK_SELECTION_NONE... which was a > > workaround because ExoIconView is broken theme-wise and sometimes has the same > > color text as selection color in some themes. Go ahead and fix the selection > > mode, but while you're at it, find the bug relating to the theme issues and > > reopen it... and finding a fix would be nice too ^_^. > > I asked Benny about this and he said that GtkCellRendererText worked fine with > ExoIconView. ExoIconView passes the flags GTK_CELL_RENDERER_SELECTED, > GTK_CELL_RENDERER_PRELIT and GTK_CELL_RENDERER_INSENSITIVE to > gtk_cell_renderer_render() depending on the state of each item. So IMHO > ExoIconView is not broken. > > Isn't it the cell renderer's job to take care of the rendering of the > background of a cell? It looks like this is not done properly in GTK+ either. > From the source of GtkIconView (GTK+ 2.14.4): (snip) I've implemented a similar hack in ExoIconView locally and it works quite well. Here's a short video: http://lunar-linux.org/~jannis/videos/xfce/xfce4-settings-manager-20081110-1.ogv Note that the icon prelighting works without hacks. All it needs is to set the "follow-state" property of the GtkCellRendererPixbuf to TRUE. The background rectangle for the text renderer painted in the hack could be modified to look similar to the rectangle Thunar paints around text. What do you think?
Did you try with different themes? Unfortunately, the caption is white on white here so selection does not show. This happens with xfce themes but also with the regular gtk theme and all the themes I tried.
(In reply to comment #4) > Did you try with different themes? > > Unfortunately, the caption is white on white here so selection does not show. > This happens with xfce themes but also with the regular gtk theme and all the > themes I tried. Well, without the background paint modification to ExoIconView I have the same white on white problem as you have an most of the themes (if not on all). The modification should work with all themes though.
Right, the issue was that GtkCellRendererText doesn't support follow-state; see bug #4305. Nick posted a patch that copies over ThunarTextRenderer that supports follow-state properly. I guess we could just use that, but I hate copy-pasting all that code just for something this stupid. Jannis, if you can hack around it in a simpler way, let's do that. Otherwise maybe look at Nick's patch here: http://foo-projects.org/~nick/patches/xfce4-settings-manager.patch If we do take ThunarTextRenderer, I'd suggest stripping out all the editing-related hooks since we're not using them.
(In reply to comment #6) > Right, the issue was that GtkCellRendererText doesn't support follow-state; see > bug #4305. Nick posted a patch that copies over ThunarTextRenderer that > supports follow-state properly. I guess we could just use that, but I hate > copy-pasting all that code just for something this stupid. > > Jannis, if you can hack around it in a simpler way, let's do that. Otherwise > maybe look at Nick's patch here: > http://foo-projects.org/~nick/patches/xfce4-settings-manager.patch The patch for libexo is just a few lines, maybe a bit more if we make the text background look like in Thunar. But it has to go into ExoIconView, not into xfce4-settings, together with a FIXME hint (since we'd be basically adding the same workaround as GTK+ does for GtkIconView). Personally, I'd prefer this method because it saves us from duplicating code. It even increases the compatibility of ExoIconView with GtkIconView. > If we do take ThunarTextRenderer, I'd suggest stripping out all the > editing-related hooks since we're not using them. Yes, if we use ThunarTextRenderer, we should do that.
I'd say add the GtkIconView-esque hack to ExoIconView -- assuming you've tested it with thunar, since ThunarTextRenderer will return TRUE for GTK_IS_CELL_RENDERER_TEXT()... Benny, this is your chance to object and yell at us for considering adding stupid hacks :-/
I'd add a boolean field to the cell info to avoid having to perform the class check every time. But other than that, there's nothing wrong with the hack, given that GTK is buggy and unlikely to be fixed anytime soon.
(In reply to comment #9) > I'd add a boolean field to the cell info to avoid having to perform the class > check every time. But other than that, there's nothing wrong with the hack, > given that GTK is buggy and unlikely to be fixed anytime soon. Done (with the boolean field): Author: jannis Date: 2008-11-11 00:17:24 +0000 (Tue, 11 Nov 2008) New Revision: 28717 Modified: libexo/trunk/ChangeLog libexo/trunk/exo/exo-icon-view.c Log: * exo/exo-icon-view.c: Add hack for drawing the background of text renderers inside the icon view. GtkIconView uses a similar hack, but this should really be fixed in GtkCellRendererText.
However, I will probably use a simplified copy of ThunarTextRenderer in xfce4-settings-manager because it has the advantage of underlined text and the "follow-state" property. It can't hurt to have the hack I just committed in ExoIconView, just in case someone wants to use it but comes across the same problem.
Done: Author: jannis Date: 2008-11-11 00:34:16 +0000 (Tue, 11 Nov 2008) New Revision: 28719 Added: xfce4-settings/trunk/xfce4-settings-manager/xfce-text-renderer.c xfce4-settings/trunk/xfce4-settings-manager/xfce-text-renderer.h Modified: xfce4-settings/trunk/xfce4-settings-manager/Makefile.am xfce4-settings/trunk/xfce4-settings-manager/xfce-settings-manager-dialog.c Log: Add XfceTextRenderer which is a copy of ThunarTextRenderer just without the edit functionality. Use this renderer in the main settings dialog so that we now have full prelit/follow-state and keyboard navigation support. This fixes bug #4593.