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 323956 - Unwanted accelerate key in the action-based toolbar.
Unwanted accelerate key in the action-based toolbar.
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: UIManager / Actions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 169252 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-12-13 08:33 UTC by Yang Hong
Modified: 2006-05-08 18:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot of Nautilus with unwanted accelerate key (64.12 KB, image/png)
2005-12-13 08:34 UTC, Yang Hong
  Details
Patch (against CVS) to remove accelerator even when they occur in middle of string (1.21 KB, patch)
2006-04-19 08:18 UTC, Abel Cheung
none Details | Review
Small test cases for above patch (1.97 KB, text/x-csrc)
2006-04-20 06:41 UTC, Abel Cheung
  Details

Description Yang Hong 2005-12-13 08:33:15 UTC
set to locale to cjk. (zh_CN.UTF-8, zh_TW.UTF-8, jp_JP.UTF-8, ko_KR.UTF-8),

start a GtkAction-based application, such as nautilus, gthumb

then you will accelerate key such as "(B)" in nautilus toolbar "Back" button,
and "(F)" in the "Forward" button and so on.

I've tested some other none-cjk locale without this problem: Italy, German, French.

I'll add a screenshot in the follow.
Comment 1 Yang Hong 2005-12-13 08:34:36 UTC
Created attachment 55934 [details]
screenshot of Nautilus with unwanted accelerate key
Comment 2 Yang Hong 2005-12-13 09:22:30 UTC
more detail:

in gtk_tool_button_construct_contents (GtkToolItem *tool_item), there is a
function called _gtk_toolbar_elide_underscores (label_text) used to elide the 
underline("_") of the label text for toolbar button, this is just suite for
none-cjk translation.

such as "_Forward" of Nautilus in the it.po:

   #: ../src/nautilus-navigation-window-menus.c:497
   msgid "_Forward"
   msgstr "_Avanti"

at this time, simple remove the "_" in the translated string is correct, but it
is not suite for CJK translate, such as zh_CN:

   #: ../src/nautilus-navigation-window-menus.c:497
   msgid "_Forward"
   msgstr "前进(_F)"

we must remove all the "(_F)" from the translate string for toolbar item.

"gtk/gtktoolbutton.c" line 245

   static void
   gtk_tool_button_construct_contents (GtkToolItem *tool_item)
   {
<...clip>

             if (button->priv->label_text)
               {
                 label_text = button->priv->label_text;
                 elide = button->priv->use_underline;
               }
             else if (button->priv->stock_id && gtk_stock_lookup
(button->priv->stock_id, &stock_item))
               {
                 label_text = stock_item.label;
                 elide = TRUE;
               }
             else
               {
                 label_text = "";
                 elide = FALSE;
               }

             if (elide)
               label_text = _gtk_toolbar_elide_underscores (label_text);
             else
               label_text = g_strdup (label_text);

             label = gtk_label_new (label_text);

             g_free (label_text);

             gtk_widget_show (label);

"gtk/gtktoolbar.c" line 4831 --98%--                  4778,1-8      99%

   gchar *
   _gtk_toolbar_elide_underscores (const gchar *original)
   {
     gchar *q, *result;
     const gchar *p;
     gboolean last_underscore;

     if (!original)
       return NULL;

     q = result = g_malloc (strlen (original) + 1);
     last_underscore = FALSE;

     for (p = original; *p; p++)
       {
         if (!last_underscore && *p == '_')
           last_underscore = TRUE;
         else
           {
             last_underscore = FALSE;
             *q++ = *p;
           }
       }

     *q = '\0';

     return result;
   }
Comment 3 Matthias Clasen 2005-12-14 17:40:22 UTC
A fix for this needs to be coordinated with bug 104112
Comment 4 Xin Zhen 2005-12-19 06:07:01 UTC
I can't see the relationship between the two bugs. ?? :-/
Bug 104112 suggests a new syntax of defining keyboard shortcuts, right? While this one talks about label display. Label of toolbar contains useless shortcut mark, that isn't elided by _gtk_toolbar_elide_underscores(). So shall we improve_gtk_toolbar_elide_underscores()?
Comment 5 Matthias Clasen 2005-12-19 19:25:26 UTC
well, the (_F) way of displaying the shortcut is just a workaround for the missing feature thats discussed in bug 104112. 
Comment 6 Owen Taylor 2005-12-21 15:31:34 UTC
Note that bug 104112 is not applicable to CJK languages, where (for menus)
the appended (_F) is standard.
Comment 7 Abel Cheung 2005-12-21 20:47:07 UTC
*** Bug 169252 has been marked as a duplicate of this bug. ***
Comment 8 Matthias Clasen 2005-12-26 07:54:56 UTC
2005-12-26  Matthias Clasen  <mclasen@redhat.com>

	* README.in: Mention the stripping of (_F) suffixes.

	* gtk/gtktoolbar.c (_gtk_toolbar_elide_underscores): Strip a suffix of 
	the form "(_<single character>)", since this is the preferred way
	for some languages to indicate accelerators.  (#323956, Yang Hong)
Comment 9 Yang Hong 2005-12-28 03:00:45 UTC
Thx Matthias :)

and I hope this fix could be merged into gtk-2-8 branche. many distributions still use gtk+-2.8.
Comment 10 Yang Hong 2005-12-28 07:56:19 UTC
unfortunately, this bug still stands in the way.:( 

I've tried to hack on _gtk_toolbar_elide_underscores, just do what you did in CVS HEAD, but failed. so I think _gtk_toolbar_elide_underscores is not the only place/way  to handle the mnemonic mark in the toolbar item,  from GtkActionEntry, from stock, etc, I was confused.:(

"gtk/gtktoolbutton.c" line 331
     if (style == GTK_TOOLBAR_TEXT && button->priv->label_widget == NULL &&
         button->priv->stock_id == NULL && button->priv->label_text == NULL)
       {

...

IMHO, if there is label_widget already, _gtk_toolbar_elide_underscores could never be called. so it's not enough to hack _gtk_toolbar_elide_underscores.
Comment 11 Matthias Clasen 2005-12-28 08:07:04 UTC
If a label_widget is set, it is the responsibility of whoever set it, to ensure that its label is appropriate for a toolbar. 
The elide_underscores hack is just for stock texts.
Comment 12 Abel Cheung 2006-04-19 08:18:12 UTC
Created attachment 63852 [details] [review]
Patch (against CVS) to remove accelerator even when they occur in middle of string
Comment 13 Abel Cheung 2006-04-19 08:19:35 UTC
Unfortunately, I have just found a case where removing (_<char>) is not enough. For example, rhythmbox use this label in toolbar:

"_Create Audio CD..."

For quite a lot of Asian languages, the standard practice is to translate to

"blahblah(_C)..."

Since the accelerator doesn't occur at end of string, it is not stripped by _gtk_toolbar_elide_underscores(). Perhaps something along the line of attachment #63852 [details] can be used? I have tested it on some locales and no apparent problem so far, though the testing platform is using gtk+ 2.8.x instead of CVS head.

OTOH, as Yang Hong said, is it appropriate to backport it to 2.8 branch? AFAIK 2.8.x is still in widespread use today.
Comment 14 Matthias Clasen 2006-04-19 13:38:53 UTC
I think what we need first is a little test application that 
runs the proposed elide function on a number of strings like

__ underscore
_abracadabra
foo_bar
chinese (_c)
bla [_b]
more (_x) bla

and check that the result is as expected in all cases
Comment 15 Abel Cheung 2006-04-20 06:41:47 UTC
Created attachment 63917 [details]
Small test cases for above patch

Some test cases to check if proposed patch can correctly remove
accelerators in the form of (_<char>), especially when
it occurs in the middle of string
Comment 16 Abel Cheung 2006-04-23 16:10:04 UTC
So far I think almost all cases are covered. However, it is a bit unfortunate that I can't optimize it -- it has to be searched char by char.
If it only occurs at the end of label, then existing change in CVS is fine.
Comment 17 Sebastien Bacher 2006-04-23 17:06:32 UTC
Ubuntu bug requesting we backport that patch for dapper: https://launchpad.net/distros/ubuntu/+source/gtk+2.0/+bug/40952
Comment 18 Matthias Clasen 2006-05-08 18:10:25 UTC
2006-05-08  Matthias Clasen  <mclasen@redhat.com>

	* gtk/gtktoolbar.c (_gtk_toolbar_elide_underscores): Elide (_x) in the middle
	of the string, too.  (#323956, Abel Cheung)