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 145346 - Use GTK+ 2.10's GtkCellRendererAccel
Use GTK+ 2.10's GtkCellRendererAccel
Status: RESOLVED OBSOLETE
Product: gnome-control-center
Classification: Core
Component: [obsolete] Keybinding
git master
Other All
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 153272 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2004-07-03 12:38 UTC by Christian Rose
Modified: 2010-11-18 18:28 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
Screenshot showing the untranslated key names in the keyboard shortcuts dialog (61.40 KB, image/png)
2004-07-03 12:40 UTC, Christian Rose
  Details
port to GtkCellRendererAccel (9.32 KB, patch)
2007-01-14 15:32 UTC, Jens Granseuer
reviewed Details | Review
support translated labels in the egg renderer (4.70 KB, patch)
2007-02-03 12:37 UTC, Jens Granseuer
committed Details | Review
gnomecc-gtkaccelrenderer-2.patch (57.18 KB, patch)
2007-02-16 15:17 UTC, Bastien Nocera
none Details | Review

Description Christian Rose 2004-07-03 12:38:00 UTC
Tested with sv_SE.UTF8.

The key names in the keyboard shortcuts control center capplet appear untranslated.

This is a problem since for example "Shift" would be translated as "Skift" in
Swedish, "Control" as "Steuerung", etc. "Ctrl" is always "Strg" on German
keyboards, so having "Control" in an otherwise German desktop makes no sense.
Also, the direction keys like "Up", "Down" etc. would need translations.

In short, the key names need translations. See also bug 60590 that made key
names translatable in GTK+.
Comment 1 Christian Rose 2004-07-03 12:40:07 UTC
Created attachment 29183 [details]
Screenshot showing the untranslated key names in the keyboard shortcuts dialog
Comment 2 Sebastien Bacher 2005-12-31 16:26:30 UTC
Reassigning to libegg, the eggaccelerators.c has that part of code
Comment 3 Sebastien Bacher 2005-12-31 16:29:14 UTC
*** Bug 153272 has been marked as a duplicate of this bug. ***
Comment 4 Christian Neumair 2006-01-01 23:10:43 UTC
I'm taking this one.
Comment 5 Christian Neumair 2006-01-02 17:44:45 UTC
Hrm egg_virtual_accelerator_name roughly seems to serve the same purpose as _gtk_accel_label_class_get_accelerator_label. I also wonder why EggVirtualModifierType is used instead of GdkModifierType.
Comment 6 Vincent Untz 2006-07-29 13:49:54 UTC
We don't want to fix this in the deprecated egg code, but in GTK+.
Comment 7 Matthias Clasen 2006-08-05 19:26:14 UTC
Well, porting the keybinding capplet from eggaccelerators to the corresponding
apis in gtk 2.10 should take care of this.
Comment 8 Bastien Nocera 2006-09-22 14:15:30 UTC
Docs there:
http://developer.gnome.org/doc/API/2.0/gtk/GtkCellRendererAccel.html
Comment 9 Jens Granseuer 2007-01-14 15:32:14 UTC
Created attachment 80240 [details] [review]
port to GtkCellRendererAccel

(Christian, I suppose you weren't working on this any more?)

This is a first attempt (against a modified 2.17.5, I hope it applies against trunk...) at porting the keybindings capplet to GtkCellRendererAccel. It works fine for me, but there is one aspect I'd like to get some opinions about:

The gtk_accelerator_{parse,name,label) functions do not support raw keycodes like the egg version did. I can see two ways to deal with this:

1. copy gtk_accelerator_name and add support for raw keycodes in cc
2. throw out support for keys for which we get no keyval

Option 2 would of course be the cleaner approach, but I'm not sure of the impact that would have. The gtk docs mention it should be unlikely, fwiw. Comments?
Comment 10 Thomas Wood 2007-01-14 20:17:46 UTC
I've tried the patch, and can no longer assign any of the "multimedia" keys on my keyboard to anything, so I think option 2 is probably out of the question. Is this something that could be fixed in GTK+?
Comment 11 Matthias Clasen 2007-01-15 02:16:35 UTC
the right fix for this is to fix the keyboard description for your keyboard, so that all keys produce proper keysyms.
Comment 12 Bastien Nocera 2007-01-15 10:01:43 UTC
(In reply to comment #11)
> the right fix for this is to fix the keyboard description for your keyboard, so
> that all keys produce proper keysyms.

Matthias, that's something I already said wasn't reasonable when you ported GtkCellRendererAccel...
Comment 13 Jens Granseuer 2007-01-15 16:30:45 UTC
What are our options, then?

1) fix gtk to support hardware keycodes
2) keep the egg renderer and beef it up to support translated keysyms

I don't know how likely option 1 is to happen, but from the comments so far I suspect the answer is not (so much for dumping libegg if this were true). Am I wrong? Is there some other way I'm missing? Or should I start hacking the egg stuff? (or do we just let this rot?)
Comment 14 Matthias Clasen 2007-01-16 01:06:26 UTC
I'm not categorically opposed to adding this ugly workaround if a patch shows up,
but it is kind of sad that we seem to be unable to make any progress on the root cause of this problem  - I wrote an xkb description for the keyboard of my laptop last year. It took some time, but it wasn't terribly hard...
Comment 15 Danilo Segan 2007-01-16 19:36:30 UTC
(In reply to comment #14)
> ... I wrote an xkb description for the keyboard of my
> laptop last year. It took some time, but it wasn't terribly hard...

Actually, I think "svu" would be more than happy to help with anyone working on that (he's maintainer of our keyboard language switcher capplet and applets, and also of xkeyboard-config which includes keyboard definitions: releases are frequent, and they hit distributions pretty fast).
Comment 16 Jens Granseuer 2007-01-25 21:01:39 UTC
(In reply to comment #14)
> I'm not categorically opposed to adding this ugly workaround if a patch shows
> up,

Ok, so I'm working on the ugly workaround now. Obviously, we cannot use _MODE_GTK for the cell renderer because that only accepts proper keysyms. However, we can't use _MODE_OTHER, either, since that doesn't translate the key names.

The two options I can see now are
1) change _MODE_OTHER to translate key names
2) add a new mode

Which is the way to go?
Comment 17 Matthias Clasen 2007-01-29 17:59:36 UTC
I don't think MODE_GTK is concerned with keycodes having or not having keysyms. 
It is concerned with making sure that the resulting accelerator combinations make sense as GTK+ accelerators. 

Isn't it enough to change gtk_accelerator_name(), gtk_accelerator_parse()
and _gtk_accel_label_class_get_accelerator_label() to handle keyvals without
a name ?
Comment 18 Jens Granseuer 2007-01-29 18:54:54 UTC
I'll try to summarize the IRC discussion I just had with Matthias:

It looks like raw keycode support won't be accepted into GTK+ after all. What the keybindings capplet should do instead to solve this properly is detect raw keycodes, ask the user for the desired keysym in case one is encountered, update the X server configuration accordingly, and use the newly mapped keysym then.

All in all, sounds like quite a bit more work than originally envisioned. In the meantime, we can think about whether we want to just change the egg renderer to translate keysyms. However, Matthias is probably right that this interim fix would prevent the real problem from being tackled (for a good while longer, at least).
Comment 19 Jens Granseuer 2007-02-03 12:37:22 UTC
Created attachment 81813 [details] [review]
support translated labels in the egg renderer

After some discussion, we'll probably take the quick and dirty road for now.

This patch modifies the egg renderer to support translated accels. As a bonus, it also fixes a small memory leak in the egg code.
Comment 20 Thomas Wood 2007-02-03 13:52:12 UTC
Assuming the patch is good, go ahead and commit. However, perhaps we should leave this bug open, as we would still like to use the GtkCellRendererAccel eventually?
Comment 21 Jens Granseuer 2007-02-03 14:07:56 UTC
Yes, I'd want to keep the bug open.

Two questions, though:

1) What the procedure for changes in libegg? Do we *want* to propagate this back, or do we "fork" libegg?

2) We're already in UI freeze. GTK+ labels use a different layout than the egg names (and they now get translated), so we'd probably need freeze break approval before committing?
Comment 22 Vincent Untz 2007-02-04 12:33:15 UTC
(In reply to comment #21)
> Yes, I'd want to keep the bug open.
> 
> Two questions, though:
> 
> 1) What the procedure for changes in libegg? Do we *want* to propagate this
> back, or do we "fork" libegg?

I'd put this in libegg too, but you need ask Bastien who maintained this in libegg.

> 2) We're already in UI freeze. GTK+ labels use a different layout than the egg
> names (and they now get translated), so we'd probably need freeze break
> approval before committing?

How is it different? If this is a UI change (not a string change, but really UI), you'll need approval from the release team.
Comment 23 Jens Granseuer 2007-02-04 12:43:12 UTC
(In reply to comment #22)
> How is it different? If this is a UI change (not a string change, but really
> UI), you'll need approval from the release team.

Can you define "really UI"?

What this changes is the labels in the treeview, e.g.

before: <Control>A
after: Ctrl+A (or, if translated, e.g. Strg+A)

Does that qualify as a string change only, and if so, is notification of gnome-doc and i18n required (since they don't have any strings to adapt. They are all coming from GTK)?
Comment 24 Vincent Untz 2007-02-04 13:25:47 UTC
This would be a string change to me. It requires notification to i18n if the strings are translatable. I'll notify documentation people about this anyway since this might affect the documentation.
Comment 25 Jens Granseuer 2007-02-04 14:02:17 UTC
Ok, fixed the doc blurb for the new function and checked it in. Mail sent to doc and i18n lists.

Bastien, let me know what to do with libegg.
Comment 26 Bastien Nocera 2007-02-04 21:34:05 UTC
(In reply to comment #25)
> Bastien, let me know what to do with libegg.

I didn't maintain that code, jrb did, I merely checked in my bug fixes (which Jonathan checked).
Comment 27 Bastien Nocera 2007-02-16 14:21:53 UTC
Adding support for keys without keysyms to GTK+ is the only way to solve this properly. Otherwise, we'd need to modify every application that captures keys to be able to use them.
Comment 28 Bastien Nocera 2007-02-16 15:10:23 UTC
(In reply to comment #27)
> Adding support for keys without keysyms to GTK+ is the only way to solve this
> properly. Otherwise, we'd need to modify every application that captures keys
> to be able to use them.

Or do as Matthias mentioned in that the X configuration should be updated to map the keycode to a specific keysym.

Sergey, is there an easy way to do this with the current libgnomekbd/ui code?
Comment 29 Bastien Nocera 2007-02-16 15:17:02 UTC
Created attachment 82682 [details] [review]
gnomecc-gtkaccelrenderer-2.patch

Updated version of Jens' patch
Comment 30 Sergey V. Udaltsov 2007-03-17 01:03:32 UTC
Bastien, I am not sure what exactly you want. Do you want some hardcoded keycode->keysym mapping? I would strongly object to that idea. Or is it something different?
Comment 31 Bastien Nocera 2007-03-19 00:38:58 UTC
(In reply to comment #30)
> Bastien, I am not sure what exactly you want. Do you want some hardcoded
> keycode->keysym mapping? I would strongly object to that idea. Or is it
> something different?

Hard-coded keycode->keysyms, but by updating the current/modifyng the current keymap. Which would allow users to create custom keyboard mappings easily.
Comment 32 Bastien Nocera 2008-07-24 15:14:23 UTC
Sergey, any updates on this?
Comment 33 Sergey V. Udaltsov 2010-05-24 21:03:59 UTC
Thanks to Thomas, who pointed me to that bug. Apologies for the silence.

I am not excited about hardcoding, but otherwise it might just work. Is the patch still applicable? 3 years is a long way...
Comment 34 Bastien Nocera 2010-11-18 18:28:06 UTC
The original reason why we wanted to fix this problem was because we wanted translations in the UI for key combinations.

Running the current gnome-control-center in the de_DE.UTF-8 locale shows logout (Ctrl+Alt+Del) showing up as "Strg+Alt+Entf".

So it seems to work properly.