GNOME Bugzilla – Bug 640389
Added support for reading pixbuf descriptions
Last modified: 2018-04-15 00:19:50 UTC
Created attachment 179142 [details] [review] Update image description based on pixbuf description This bug report is a split of the original bug filed as 617629, "CellRendererPixbuf not accessible to the blind". This report's patch effects only gail source code in modules/other. To compile using this patch, first apply the patch in 617629, so that pixbuf objects will have a description property. This patch causes gail image cells to inherit descriptions from their pixbuf objects, enabling Orca to read them. The patch was generated using "git diff HEAD^", and was applied successfully to commit b42b47e7d37411d7410113aa8156146d35dfde43 of the gtk+ git repository.
One comments, why don't use GNOME string functions like g_strcmp0?
Simply because of my lack of experience in Gnome/GTK+ programming. Would you like me to change to GNOME string functions and re-submit the patch? Thanks.
Review of attachment 179142 [details] [review]: patch needs work. ::: modules/other/gail/gailimagecell.c @@ +117,3 @@ + g_object_get (G_OBJECT(cell->renderer), "pixbuf", &pixbuf, NULL); + if(!GDK_IS_PIXBUF (pixbuf)) + return FALSE; change the type check with a simple NULL check; the property type will make sure that it's either NULL or a GdkPixbuf. @@ +118,3 @@ + if(!GDK_IS_PIXBUF (pixbuf)) + return FALSE; + gchar *new_description = NULL; need a space between the function name and the (), as per coding style. @@ +126,3 @@ + */ + if (new_description == NULL || + return FALSE; you can change this to g_strcmp0() which does NULL checking for you @@ +134,3 @@ + } + } + /* a space is needed between if and (), as per coding style. @@ +136,3 @@ + else if(new_description) + rv = TRUE; + * If the new value is NULL and the old value isn't NULL, then the this whole block can be effectively changed to: retval = FALSE; if (g_strcmp0 (image_cell->image_description, new_desc) != 0) { g_free (image_cell->image_description); image_cell->image_description = g_strdup (new_desc); retval = TRUE; } g_free (new_desc); @@ +142,3 @@ + { + image_cell->image_description = NULL; + if (new_description == NULL || no need to use braces for a single statement. @@ +147,3 @@ + image_cell->image_description = g_strdup (new_description); + } + g_free (image_cell->image_description); no need for this whole block, actually: g_strdup() returns NULL if the string you're copying is NULL.
As I said in the other bug, I don't think attaching a description to the pixbuf is a good approach. Pixbufs are just a bunch of image data, with no particular relevance, and in GTK+ apis they are often used interchangeably with icon-names, GIcons or cairo surfaces. I'd much rather see an alt-text property on GtkImage and GtkCellRendererPixbuf and the various other places where GTK+ widgets display images.
I get really pissed off when I hear people coming up with reasons to do nothing about fixing accessibility problems in Linux. Instead of nit picking, how about just getting the pixbuf cell renderers to talk? Also, I believe you're wrong about pixbuf descriptions being a poor solution. In reality, we need pragmatic solutions for accessibility, not elegant solutions that will never be used. Programs I've looked through mostly build pixbufs and later select which one to render in a cell. We need a one-line solution that allows existing programs to attach text to these objects. A pixbuf description is the simplest, most pragmatic solution, just not an elegant or complete solution. GTK+ has HUGE accessibility gaps. There has been little improvement in several years. I've got a fix for just one of these problems, and what I'm hearing is I need to go rewrite it differently, reformat it, split the bug into parts, come up with a more elegant solution, etc. How about a little actual action? Li seems to be doing good work in gail land, but he can't fix things like pixbuf cells, because GTK+ doesn't provide any way for a programmer to get him the text he needs. These cells don't talk, and it's not Li's fault, nor the application developer's fault. It's a bug in GTK+. Ideally, someone should be able to simply file an accessibility bug, and in short order, it would get fixed. I'd be more willing to spend more of my time on GTK+ accessibility if I believed someone in GTK+ land is also willing to work on improving accessibility. There are a lot more accessibility problems in GTK+ than just pixbuf cell rendering. Combo boxes need to let us pick an entry by typing the first letters. We need to be able to hear grayed out menu items. The icons in the Gnome panels need to talk. There's tons of work to do, and frankly we aren't making much progress so far as I can tell. So... I've removed all packages in Vinux that improve GTK+ accessibility, and I wont be working further on this or any other GTK+ issues unless something changes in GTK+ land. You win. You wont have to do any work on accessibility.
> I get really pissed off when I hear people coming up with reasons to do nothing > about fixing accessibility problems in Linux. Instead of nit picking, how > about just getting the pixbuf cell renderers to talk? You are entitled to your opinion. But bitching at me here is not going to help your cause.
You are right, and I'm sorry for the harsh tone of my last post. You are not personally responsible for the slow state of GTK+ accessibility improvement.
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new