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 504246 - use the stock item's label in gailimage
use the stock item's label in gailimage
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Accessibility
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-12-18 14:03 UTC by Christian Persch
Modified: 2007-12-20 00:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (12.28 KB, patch)
2007-12-18 14:51 UTC, Christian Persch
needs-work Details | Review
updated patch (12.96 KB, patch)
2007-12-18 20:15 UTC, Christian Persch
accepted-commit_now Details | Review

Description Christian Persch 2007-12-18 14:03:35 UTC
(There's no 'gail' component yet, filing in 'general'.)

gailimage.c has its own table of strings to use for stock images, but the strings are identical to the stock item's label except they're all-lowercase and without mnemonics. I think we should remove that, and just use the stock item label, stripping the mnemonic like we do in the toolbar code (I don't think the lowercase matters?).
Comment 1 Matthias Clasen 2007-12-18 14:05:53 UTC
Yes, please.
Comment 2 Christian Persch 2007-12-18 14:51:28 UTC
Created attachment 101190 [details] [review]
proposed patch

Turns out not all labels were /exactly/ the same as the stock item label:

Stock item "gtk-dialog-info" gailimage.c "dialog information" transformed stock label "information"
Stock item "gtk-dialog-warning" gailimage.c "dialog warning" transformed stock label "warning"
Stock item "gtk-dialog-error" gailimage.c "dialog error" transformed stock label "error"
Stock item "gtk-dialog-question" gailimage.c "dialog question" transformed stock label "question"
Stock item "gtk-cdrom" gailimage.c "cdrom" transformed stock label "cd-rom"
Stock item "gtk-goto-bottom" gailimage.c "go to bottom" transformed stock label "bottom"
Stock item "gtk-goto-first" gailimage.c "go to first" transformed stock label "first"
Stock item "gtk-goto-last" gailimage.c "go to last" transformed stock label "last"
Stock item "gtk-goto-top" gailimage.c "go to top" transformed stock label "top"
Stock item "gtk-go-back" gailimage.c "go back" transformed stock label "back"
Stock item "gtk-go-down" gailimage.c "go down" transformed stock label "down"
Stock item "gtk-go-forward" gailimage.c "go forward" transformed stock label "forward"
Stock item "gtk-go-up" gailimage.c "go up" transformed stock label "up"
Stock item "gtk-justify-center" gailimage.c "justify center" transformed stock label "center"
Stock item "gtk-justify-fill" gailimage.c "justify fill" transformed stock label "fill"
Stock item "gtk-justify-left" gailimage.c "justify left" transformed stock label "left"
Stock item "gtk-justify-right" gailimage.c "justify right" transformed stock label "right"
Stock item "gtk-revert-to-saved" gailimage.c "revert to saved" transformed stock label "revert"
Stock item "gtk-select-color" gailimage.c "select color" transformed stock label "color"
Stock item "gtk-select-font" gailimage.c "select font" transformed stock label "font"
Stock item "gtk-sort-ascending" gailimage.c "sort ascending" transformed stock label "ascending"
Stock item "gtk-sort-descending" gailimage.c "sort descending" transformed stock label "descending"
Stock item "gtk-zoom-100" gailimage.c "zoom 100 percent" transformed stock label "normal size"
Stock item "gtk-zoom-fit" gailimage.c "zoom fit" transformed stock label "best fit"

And there were a few stock IDs present in the gailimage table which gtk_stock_lookup doesn't find:

Stock item "gtk-dnd" not found
Stock item "gtk-dnd-multiple" not found
Stock item "gtk-color-picker" not found
Stock item "gtk-missing-image" not found
[+ many of the "gnome-*" stock icons]

I think I still prefer this patch, since it will also work with the stock items that programmes define themselves.
Comment 3 Christian Persch 2007-12-18 14:56:22 UTC
The one problem is that atk_object_get_name returns |const char*| while I have a to-be-freed string when I take the stock item's label and transform it. Setting it back with atk_object_set_name works, but then the image doesn't change name when the stock ID changes...
Comment 4 Matthias Clasen 2007-12-18 15:14:18 UTC
instead of calling atk_object_set_name, you could keep the modified string in private, and update it when the stock id changes. 
Comment 5 Matthias Clasen 2007-12-18 15:14:43 UTC
Alternatively, intern the string
Comment 6 Christian Persch 2007-12-18 20:15:06 UTC
Created attachment 101213 [details] [review]
updated patch
Comment 7 Matthias Clasen 2007-12-19 15:47:20 UTC
so, this stores the name in the accessible, but still reconstructs it every time ?
that seems a bit wasteful. i guess with a bit more work, we could return the stored name instead.
Comment 8 Christian Persch 2007-12-19 18:33:56 UTC
Yes, I only store it because I need to return a const string, so I always reconstruct it to get the up-to-date string (stock ID change).
The alternative would be to listen to notify::stock and update the string there... I wasn't sure the minor speedup of atk_object_get_name is worth a signal connection.
Comment 9 Matthias Clasen 2007-12-19 18:52:34 UTC
Yeah, lets not worry about that for now. 
Comment 10 Christian Persch 2007-12-20 00:26:06 UTC
        * modules/other/gail/Makefile.am:
        * modules/other/gail/gailimage.c: (gail_image_class_init),
        (gail_image_init), (elide_underscores), (gail_image_get_name),
        (atk_image_interface_init), (gail_image_finalize):
        * modules/other/gail/gailimage.h:
        R modules/other/gail/gailintl.h:
        * po/POTFILES.skip: Use the stock item's label as the name of the
        accessible in gailimage.c. Bug #504246.