(as a workaround, go into the control center, click "Keyboard", click "Layouts" tab, click "Layout Options", open "Compose Key Position", and assign Compose to some key. (I use "Menu", which is the key next to the right Windows key.) Then you can type "Compose+' a" to get á, etc.)

http://live.gnome.org/GnomeShell/Design/Guidelines/SystemStatus/InputLanguage

FYI.
Currently I'm discussing IBus XKB feature with IBus maintainers.
It might be nice if I could get any feedback.

http://desktopi18n.wordpress.com/2010/10/06/ibus-xkb/
http://pkgs.fedoraproject.org/gitweb/?p=ibus.git;a=blob_plain;f=ibus-541492-xkb.patch;hb=HEAD

Thanks.

Created attachment 177526 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd.

Created attachment 177527 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd.

Removing a file that slipped in the patch.

From libgnomekbd POV the patch is perfecly fine. I would like it to be committed to gnome-shell ASAP

I just have to mention: that XKB indicator is functional equivalent of what we have in GNOME2. The integration between IM and XKB is still a dream.

Created attachment 177529 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd.

Refactored to avoid messing with GdkPixbufs-as-GIcons

Looks good with me.
Currently I'm working on GNOME-Shell for IBus.
https://github.com/fujiwarat/ibus/tree/gjs/ui/gjs

Giovanni, there's one thing I forgot.

If the number of layouts=1, there should not be indicator - and it should automatically appear if it changes to 2 or more - unless there is "blocking" setting (currently it is "disable-indicator" in "org.gnome.settings-daemon.plugins.keyboard").

Could it be done?

There is additional stuff related to CapsLock/NumLock/ScrollLock - but I am not sure if it belongs in here, or should I open separate bug

Created attachment 177598 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd. The indicator is shown when more than one group
is loaded in X and it is not disabled in GSettings.

The part about autohiding depends on bug 638306

Could we have that committed before 2.91.5 and UI freeze please?

I'll review the patch as is in a little bit, but first wanted to tackle the question of UI. Even leaving aside ibus integration, the UI this patch presents is not the same as http://live.gnome.org/GnomeShell/Design/Guidelines/SystemStatus/InputLanguage 

Although we can land this patch as is, and get back to the GNOME 2 status quo,  in the long term, getting the right user interface isn't something that we can be flexible on. In my opinion, the current interface that is based largely on country names just isn't right. (And this has nothing to do with the flags question ... at the moment I'm happy to pretend that we could default to flags if it made the most sense.)

I spent some time looking through all the keyboard layouts in xkeyboard-config and picked out a few representative examples.

 Isl
  This is a basically a simple case. There is one country where Icelandic
  is spoken, Iceland, and if you want to write Icelandic, you need the
  Icelandic layout. You could identify the layout by country name, language
  name or flag, and it would work. (This is almost the only completely simple
  case I could find!)

 Grc
  Grc is a bit like Isl in that there is a strong 1:1 mapping between
  country and language, but the difference is that for Grc, the Greek
  layout will never be the only layout; the Greek layout will be combined
  with a Latin layout, and we need to be able to display the Latin layout
  in a reasonable fashion.

 Ara
  While otherwise like Grc, The Arabic layout doesn't correspond to any 
  country. There would be no way of showing a country name or
  flag for Arabic. The only way to indicate Arabic would be by script
  name, or probably better, with a representative character.
   
 Isr
  Isr is fairly similar to Grc, but with the twist that in addition
  to Hebrew and Latin layouts, many users will want Arabic or Cyrillic
  layouts.

 Ire
  Ireland is one of the best cases for country-based indication. There are
  two official languages in Ireland that share the same keyboard layout.
  Showing an abbreviation for "English" or "Gaelige" would be wrong.

 Ind
  On the other hand, India is a poster-child *against* country based
  indication. There's no Indian keyboard that could come close to
  entering all the different scripts of India.  (Actually, I'm sort of
  cheating on this one; xkbconfig has a bunch of different language variants
  but in real life there is one keyboard layout and the language is
  added at the input method level. Still, with the goal of ibus vs. xkb
  UI unification, the end resuls is the same.)
  
 LAm
  While there is a distinct keyboard layout for Latin America, 
  LAm is clearly nothing we want to display in a keyboard indicator.
  There is no flag or country name, so the only possible indication
  is language based - 'es'.

The claim I'm going to make is that we can't standardize on language name, and less on country name, but that each keyboard variant has a basically static short identifier that we can use in the panel that will be correct in combination with anything that is likely to be used with it.

This might be:

 Language name - Lam => es
                 USA => en

 Country name -  Ire => ie

 Representative character or characters - Isl => א

If two keyboard variants would get the same identifier - GBr s. USA for example, then they aren't really plausible to be combined as different groups in a single keyboard configuration. We need, of course, to let the user choose between them, but they'll want only one at a time.

You'd probably also want in the config the expanded form of this

 English
 Español
 Ireland / Éireann
 עברית
 
for use in the menu. I think only in the country case do you actually want translation for the expanded form, but comprehensive translation of the expanded form is probably OK too. You definitely *don't* want translation of the short abbreviations in almost all cases.

How would we go about adding this to the xkeyboard-config data?

Review of attachment 177598 [details] [review]:

One thing bothering me when testing it - the indicator changing width. The code looks good except for a tiny comment.

::: js/ui/status/keyboard.js
@@ +51,3 @@
+
+        this._iconActor = new St.Icon({ icon_name: 'keyboard', icon_type: St.IconType.SYMBOLIC, style_class: 'system-status-icon' });
+        this._labelActor = new St.Label({ text: 'USA' });

Using a St.Label here means that when you change groups the items to the left of the keyboard indicator (the accessibility menu say) move slightly because different layouts are different lengths. The simple workaround I guess would be to make sure that the keyboard indicator is always the leftmost (or for RTL rightmost) icon. But might be better to use a GenericContainer and shell_generic_container_set_skip_paint() to do it right and have labels for all the layouts present and taken into account but only the current one painted.

@@ +93,3 @@
+            this._layoutItems[i].destroy();
+
+        let short_groups = this._config.get_short_group_names();

short_names would be slightly better as a variable name than short_groups

> I'll review the patch as is in a little bit, but first wanted to tackle the
> question of UI.
Thanks, that is always interesting topic to discuss (no sarcasm attached).

> Although we can land this patch as is, and get back to the GNOME 2 status quo, 
I guess for 3.0 that's the only choice - considering that we're nearly in UI freeze. But that should not prevent us discussing plans for 3.2.

> be flexible on. In my opinion, the current interface that is based largely on
> country names just isn't right. (And this has nothing to do with the flags
> question ... at the moment I'm happy to pretend that we could default to flags
> if it made the most sense.)
Oops. I always thought of flags as "semi-legal" feature - never expected to expose it officially. But it can be raised at d-d-l as separate topic (again:). Anyway, let's not discuss them here.
  
>  Ara
>   While otherwise like Grc, The Arabic layout doesn't correspond to any 
>   country. There would be no way of showing a country name or
>   flag for Arabic. The only way to indicate Arabic would be by script
>   name, or probably better, with a representative character.
What's wrong with just "Ara"?

>  Isr
>   Isr is fairly similar to Grc, but with the twist that in addition
>   to Hebrew and Latin layouts, many users will want Arabic or Cyrillic
>   layouts.
Well, that's not a real difference IMHO. I would call that case "a country that never uses its own layout alone". Whether users need 2 or 4 extra layouts - does it matter?

>  Ire
>   Ireland is one of the best cases for country-based indication. There are
>   two official languages in Ireland that share the same keyboard layout.
>   Showing an abbreviation for "English" or "Gaelige" would be wrong.
You see now, why I am enforcing country-based approach, living in Ireland:)

>  Ind
>   On the other hand, India is a poster-child *against* country based
>   indication. There's no Indian keyboard that could come close to
>   entering all the different scripts of India.  (Actually, I'm sort of
>   cheating on this one; xkbconfig has a bunch of different language variants
>   but in real life there is one keyboard layout and the language is
>   added at the input method level. Still, with the goal of ibus vs. xkb
>   UI unification, the end resuls is the same.)
I am not sure even language-based approach is good here. Is "Deva" readable and understandable ("devangari")?

>  LAm
>   While there is a distinct keyboard layout for Latin America, 
>   LAm is clearly nothing we want to display in a keyboard indicator.
>   There is no flag or country name, so the only possible indication
>   is language based - 'es'.
That's fair. Perhaps I should change shortDescription to Esp here.
 
> The claim I'm going to make is that we can't standardize on language name, and
> less on country name, but that each keyboard variant has a basically static
> short identifier that we can use in the panel that will be correct in
> combination with anything that is likely to be used with it.
Yes, that's why I introduced short descriptions. So far they are available only for layouts, not for variants. DTD allows to add them for variants as well - it is libxklavier that ignores them currently.

> This might be:
> 
>  Language name - Lam => es
>                  USA => en
Respectfully disagree. I would prefer to use 'USA' or 'us' here. To distinguish from other English layouts.

>  Representative character or characters - Isl => א
Ugh... I could put it into shortDescription, but IMHO this is really obscure

> If two keyboard variants would get the same identifier - GBr s. USA for
> example, then they aren't really plausible to be combined as different groups
> in a single keyboard conf iguration. We need, of course, to let the user choose
> between them, but they'll want only one at a time.
I seen some configs with both GBr and USA. Not sure why people wanted that - perhaps they shared desktops or smth.

> You'd probably also want in the config the expanded form of this
> 
>  English
>  Español
>  Ireland / Éireann
>  עברית
> 
> for use in the menu. I think only in the country case do you actually want
> translation for the expanded form, but comprehensive translation of the
> expanded form is probably OK too. You definitely *don't* want translation of
> the short abbreviations in almost all cases.
Why??? I am positive that Russians do not want to see USA/Eng/... It would be more correct to see США/Англ/...
 
> How would we go about adding this to the xkeyboard-config data?
In general, this is all good. IMHO your ideas fit with existing schema, with slight extension. Let me try to summarize (and please correct me if I am wrong or missed something):
1. Ability to provide (and use) shortDescription for variants, not only layouts
2. For each layout/variant, have a flag indicating what should be displayed:
  - shortDescription, translatable
  - language (first item from languageList?), translatable
  - country (first item from countryList?), translatable
By default, shortDescription is to be used (if that flag is not specified) - for variant (if available) or layout (it is mandatory).
3. Change LAm to Esp

I must admit, I am opposed to the idea of having representative characters. IMHO this not easy to understand. And if user really wants something graphical - he can always use the flags (with tiny hack like "mv russian_glyph.png ru.png")

Actually, about shortDescription for variants. The only case I can imagine so far is India. What are other cases for descriptions on variant level? Or we could just say that Indian variants should display language instead of shortDescription. Perhaps, #1 and #2 in my list are alternatives - it would be enough to implement just one of them...

*** Bug 633010 has been marked as a duplicate of this bug. ***

Created attachment 177758 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd. The indicator is shown when more than one group
is loaded in X and it is not disabled in GSettings.

(In reply to comment #15)
(And this has nothing to do with the flags
> > question ... at the moment I'm happy to pretend that we could default to flags
> > if it made the most sense.)
> Oops. I always thought of flags as "semi-legal" feature - never expected to
> expose it officially. But it can be raised at d-d-l as separate topic (again:).
> Anyway, let's not discuss them here.

I'm certainly not suggesting anything has changed, just trying to say that potential problems with flags aren't why I'm arguing against country indication.
 
> >  Ara
> >   While otherwise like Grc, The Arabic layout doesn't correspond to any 
> >   country. There would be no way of showing a country name or
> >   flag for Arabic. The only way to indicate Arabic would be by script
> >   name, or probably better, with a representative character.
> What's wrong with just "Ara"?

I think the basic reason is that short abbreviations aren't necessarily short in other scripts - Latin is a pretty narrow script, Arabic is a pretty wide script. And we want that is short and iconic and recognizable on the panel. Something that while "text" acts like an icon in being a single visual unit.

But the reason that the representative character method is workable is that keyboard layout selection is basically *script* selection.

Example - an Urdu speaker in Pakistan will have an Urdu keyboard layout. If they want to write some Arabic (the Koran is always read in Arabic), they will also use that layout. They aren't going to switch to the completely different Arabic layout (compare http://en.wikipedia.org/wiki/Urdu_keyboard with http://en.wikipedia.org/wiki/Arabic_keyboard).

So, just as people almost never will have two layouts for the same language installed, they will seldom have two layouts for the same script installed. 

> >  Isr
> >   Isr is fairly similar to Grc, but with the twist that in addition
> >   to Hebrew and Latin layouts, many users will want Arabic or Cyrillic
> >   layouts.
> Well, that's not a real difference IMHO. I would call that case "a country that
> never uses its own layout alone". Whether users need 2 or 4 extra layouts -
> does it matter?

I'd agree that the Isr and Grc cases are similar, the main difference would be that Arabic is also an official language of Israel, so it's more obvious that the Hebrew layout is a layout for the Hebrew language or Hebrew script rather than the country.
 
> >  Ire
> >   Ireland is one of the best cases for country-based indication. There are
> >   two official languages in Ireland that share the same keyboard layout.
> >   Showing an abbreviation for "English" or "Gaelige" would be wrong.
> You see now, why I am enforcing country-based approach, living in Ireland:)
>
> >  In
> >   On the other hand, India is a poster-child *against* country based
> >   indication. There's no Indian keyboard that could come close to
> >   entering all the different scripts of India.  (Actually, I'm sort of
> >   cheating on this one; xkbconfig has a bunch of different language variants
> >   but in real life there is one keyboard layout and the language is
> >   added at the input method level. Still, with the goal of ibus vs. xkb
> >   UI unification, the end resuls is the same.)
> I am not sure even language-based approach is good here. Is "Deva" readable and
> understandable ("devangari")?

India is a pretty complex case. Most of the largest languages of India have their own scripts, but there are also various officially recognized languages that are written with Devanagari or Latin characters.

Scripts do have commonly recognized names, so I think in a menu it's fine to show Devanagari (or the translated version of it) as a long label. (There are a few other cases where the script name mismatches the predominate language written in it, but usually they are the same - the Tamil script is used to write Tamil, etc.)

But I think it would be pretty odd to show Dev or Tam as an indicator on the panel for someone running with English messages. Represenative characters like क /க would be much more meaningful.

> >  LAm
> >   While there is a distinct keyboard layout for Latin America, 
> >   LAm is clearly nothing we want to display in a keyboard indicator.
> >   There is no flag or country name, so the only possible indication
> >   is language based - 'es'.
> That's fair. Perhaps I should change shortDescription to Esp here.
> 
> > The claim I'm going to make is that we can't standardize on language name, and
> > less on country name, but that each keyboard variant has a basically static
> > short identifier that we can use in the panel that will be correct in
> > combination with anything that is likely to be used with it.
> Yes, that's why I introduced short descriptions. So far they are available only
> for layouts, not for variants. DTD allows to add them for variants as well - it
> is libxklavier that ignores them currently.
> 
> > This might be:
> > 
> >  Language name - Lam => es
> >                  USA => en
> Respectfully disagree. I would prefer to use 'USA' or 'us' here. To distinguish
> from other English layouts.

To repeat - the short "on the panel" abbreviation needs to be unique and distinguishable among the users groups, not among all layouts. There is basically no circumstance in which someone want to have two groups, one group with a us layout and one group with an uk layout.

(people do sometimes have both Dvorak and English layouts - this actually triggers problems with GTK+'s accelerator code and basically doesn't work, but for this reason, the Dvorak layout probably needs a short indicator that is based on "Dvorak")

To a American English speaker like me, seeing USA on the panel is pretty weird. Partly because the full "USA" isn't really used except in formal circumstances but more because my mental process isn't that I want to do some typing in "USA-an" (there are various parts of the US where the predominant language  ... French, Spanish, Navajo, etc. can't even be typed on a us keyboard layout!)

I can only imagine that seeing USA is even more weird to someone in most of the world who wants to write a bit of English or maybe a bit of C code and needs the Latin alphabet.

If I select the very same layout as Canada - English, then the short description would presumably be Eng (since Can defaults to being a French layout), so it's unclear why it wouldn't be indicated the same way when selected as the US layout.

> >  Representative character or characters - Isl => א
> Ugh... I could put it into shortDescription, but IMHO this is really obscure
> 
> > If two keyboard variants would get the same identifier - GBr s. USA for
> > example, then they aren't really plausible to be combined as different groups
> > in a single keyboard conf iguration. We need, of course, to let the user choose
> > between them, but they'll want only one at a time.
> I seen some configs with both GBr and USA. Not sure why people wanted that -
> perhaps they shared desktops or smth.

What I'm going to say to that is that if we make something possible via the user interface some people will do it, but I don't think just because some people do it means that it is something that should drive our design.

> > You'd probably also want in the config the expanded form of this
> > 
> >  English
> >  Español
> >  Ireland / Éireann
> >  עברית
> > 
> > for use in the menu. I think only in the country case do you actually want
> > translation for the expanded form, but comprehensive translation of the
> > expanded form is probably OK too. You definitely *don't* want translation of
> > the short abbreviations in almost all cases.
> Why??? I am positive that Russians do not want to see USA/Eng/... It would be
> more correct to see США/Англ/...

I'm not really in a position to tell you what Russians want to see :-)

In general, I'd make the point that there's no point in having a keyboard group for a language that you aren't going to type in - I may not be able to recognize what Malayalam looks like written in Malayalam but that doesn't matte. If I was a person that *did* want to write something in Malayalam, then seeing it written in Malayalam or a representative character like "അ" will be a faster and more immediate way of recognizing it than an abbrevation of the English form.

Note that the combination of:;

 Native Malayalam speaker
 Malayalam keyboard layout
 English messages

Will be a very common one.

Obviously, us used as a generic Latin layout is somewhat of a special case... having that group doesn't imply that you have much familiarity with English. I don't have a great answer, I'll say that none of:

 USA/Eng/США/Англ
 
At all say "The keyboard layout I need in order to type C code" so it's basically going to be learned in any case. The "representative character" approach might work well here, but I'm not really convinced it makes sense for Latin layouts.

> > How would we go about adding this to the xkeyboard-config data?
> In general, this is all good. IMHO your ideas fit with existing schema, with
> slight extension. Let me try to summarize (and please correct me if I am wrong
> or missed something):
> 1. Ability to provide (and use) shortDescription for variants, not only layouts
> 2. For each layout/variant, have a flag indicating what should be displayed:
>   - shortDescription, translatable
>   - language (first item from languageList?), translatable
>   - country (first item from countryList?), translatable
> By default, shortDescription is to be used (if that flag is not specified) -
> for variant (if available) or layout (it is mandatory).
> 3. Change LAm to Esp

I think I'd agree that if we treat shortDescription as what to display on the panel, then by itself it's pretty much sufficient. What actually gets put in there is pretty much a detail - if we want to put <shortDescription>क</shortDescription> there for the Devanagari layout, then that will work out. (And presumably nobody will translate it, because most translators will have no idea what it should be translated to, and if somebody did know that it was Devanagari for 'ka', they defintely wouldn't translate it to Ka!)

I don't immediately see the need for the flag indicating what to use - if we want the language or country name, can't we just put that as shortDescription?

What I think might be missing is text to put in the menu along the short description - it seems to me that the <description/> is really intended for something else (for selection in the control center) and anything automatically derived from that might frequently be wrong or at least awkward.

> I must admit, I am opposed to the idea of having representative characters.
> IMHO this not easy to understand. And if user really wants something graphical
> - he can always use the flags (with tiny hack like "mv russian_glyph.png
> ru.png")

Basically, the fact that you or I find क an exceedingly cryptic way to say "keyboard group needed to input Hindi" doesn't mean that it is a bad way to indicate it.

Review of attachment 177758 [details] [review]:

Looks very close

::: js/ui/status/keyboard.js
@@ +156,3 @@
+
+    _get_preferred_width: function(container, for_height, alloc) {
+        // always require at least the size of one icon

This is (somewhat) obvious from the code, but what isn't obvious and should be in the comment is the _why_

@@ +159,3 @@
+        let [max_min_width, max_natural_width] = this._iconActor.get_preferred_width(for_height);
+
+        if (!this._showFlags) {

You should call get_preferred_width() on all the children, but can ignore the results you get back in the showFlags case

@@ +177,3 @@
+            let [min_height, natural_height] = this._labelActors[i].get_preferred_height(for_width);
+            max_min_height = Math.max(max_min_height, min_height);
+            max_natural_height = Math.max(max_natural_height, natural_height);

Shouldn't you also be taking the showFlags setting into account here?

> I'm certainly not suggesting anything has changed, just trying to say that
> potential problems with flags aren't why I'm arguing against country
> indication.
Ok. I see your point. This topic is closed without opening.

> Arabic is a pretty wide
> script. And we want that is short and iconic and recognizable on the panel.
Can we leave it to the translators of xkeyboard-config (Translation Project)?

> But the reason that the representative character method is workable is that
> keyboard layout selection is basically *script* selection.
Well, to some extent... At least this is generally true when we're talking about single user.

> So, just as people almost never will have two layouts for the same language
> installed, they will seldom have two layouts for the same script installed. 
Agree.

> Scripts do have commonly recognized names, so I think in a menu it's fine to
> show Devanagari (or the translated version of it) as a long label. (There are a
> few other cases where the script name mismatches the predominate language
> written in it, but usually they are the same - the Tamil script is used to
> write Tamil, etc.)
I guess currently we write "India - Devangari". I think it is good enough.

> But I think it would be pretty odd to show Dev or Tam as an indicator on the
> panel for someone running with English messages. Represenative characters like
> क /க would be much more meaningful.
Err... It is hard for me to tell something here - I do not know any of those languages. I will try to find someone who knows and uses them daily.

> To repeat - the short "on the panel" abbreviation needs to be unique and
> distinguishable among the users groups, not among all layouts. 
Oh, ok, I see now. Yes, you're making a good point. Except that here we will have to predict that are possible and impossible combinations for one user.
 
> I can only imagine that seeing USA is even more weird to someone in most of the
> world who wants to write a bit of English or maybe a bit of C code and needs
> the Latin alphabet.
As a matter of fact, you're right. I heard some complains from Russians.

> matte. If I was a person that *did* want to write something in Malayalam, then
> seeing it written in Malayalam or a representative character like "അ" will be a
> faster and more immediate way of recognizing it than an abbrevation of the
> English form.
I would not say for everybody - but for many people perhaps that is true. For some people it is easier to recognize graphical representations, for some - textual.
 
> I think I'd agree that if we treat shortDescription as what to display on the
> panel, then by itself it's pretty much sufficient. What actually gets put in
> there is pretty much a detail - if we want to put
Well, it may be a "detail" from gnome POV. But with xk-c maintainer hat on, I would be quite interested to get patches to base.xml.in;)

> that will work out. (And presumably nobody will translate it, because most
> translators will have no idea what it should be translated to, and if somebody
> did know that it was Devanagari for 'ka', they defintely wouldn't translate it
> to Ka!)
That is a funny situation. What you're saying is that some shortDescriptions are going to translatable - some are not.

> I don't immediately see the need for the flag indicating what to use - if we
> want the language or country name, can't we just put that as shortDescription?
Yes, that's exactly what I meant in my last comment. Let's just expand usage of shortDescription.

> What I think might be missing is text to put in the menu along the short
> description - it seems to me that the <description/> is really intended for
> something else (for selection in the control center) and anything automatically
> derived from that might frequently be wrong or at least awkward.
The description is exactly the text to put into menu. Why don't you like it? Actually it is the same as group name in the symbols file.

Ok, you nearly convinced me that glyphs can be used (again, as long as we have functioning stack in place, we can patch base.xml.in endlessly).

So, as gnome dev, I'll try to make the stack with shortDescriptions work as we discussed. But as xk-c keeper, I need people to update base.xml.in...

(In reply to comment #20)
> Review of attachment 177758 [details] [review]:
> 
> Looks very close
> 
> ::: js/ui/status/keyboard.js
> @@ +156,3 @@
> +
> +    _get_preferred_width: function(container, for_height, alloc) {
> +        // always require at least the size of one icon
> 
> This is (somewhat) obvious from the code, but what isn't obvious and should be
> in the comment is the _why_

I'm not really sure, just it seemed that for consistency with the other indicators, it should be at least as large as the other (for example in a11y themes). Anyway this does not hold much, because the font will be scaled proportionally in any case, so I removed the code and the comment.

> 
> @@ +159,3 @@
> +        let [max_min_width, max_natural_width] =
> this._iconActor.get_preferred_width(for_height);
> +
> +        if (!this._showFlags) {
> 
> You should call get_preferred_width() on all the children, but can ignore the
> results you get back in the showFlags case

Really?
Is it some sort of constraint in Clutter to always propagate get_preferred_width/height?

> 
> @@ +177,3 @@
> +            let [min_height, natural_height] =
> this._labelActors[i].get_preferred_height(for_width);
> +            max_min_height = Math.max(max_min_height, min_height);
> +            max_natural_height = Math.max(max_natural_height, natural_height);
> 
> Shouldn't you also be taking the showFlags setting into account here?

Fixed.

Created attachment 177955 [details] [review]
Status Area: add keyboard layout selector

Add an indicator for the current keyboard layout, based on
libgnomekbd. The indicator is shown when more than one group
is loaded in X and it is not disabled in GSettings.

Can we have it in the current form in 9.21.5 - at least to have something before UI freeze?

Actually, there is one extra bit missing: the LED indicators. The icons were created by Jimmac some while ago (see bug #616998). I updated GkbdConfiguration to provide necessary events and state info. Giovanni, could you please incorporate? Thank you.

Sorry, I forgot to mention: LED indicators are optional (only some people, with LEDless keyboards need them). This is managed by settings key "org.gnome.settings-daemon.plugins.keyboard":"show-keyboard-leds-indicator"

Created attachment 178147 [details] [review]
KeyboardStatus: handle modifier key indicators

Introduce a generic framework for on/off indicators that are shown
in the panel, next to the system status area, and use it for
showing the status of modifier keys.

You're genius:) Looking forward to pull all those bits from git

(In reply to comment #22)

> > @@ +159,3 @@
> > +        let [max_min_width, max_natural_width] =
> > this._iconActor.get_preferred_width(for_height);
> > +
> > +        if (!this._showFlags) {
> > 
> > You should call get_preferred_width() on all the children, but can ignore the
> > results you get back in the showFlags case
> 
> Really?
> Is it some sort of constraint in Clutter to always propagate
> get_preferred_width/height?

I'm actually not completely sure about Clutter - I know in GTK+ widgets are allowed to assume that size_request() will be called before size_allocate(). Even if that isn't documented as standardized for Clutter, since it *normally* happens, people will probably make implicit assumptions in widget implementations that it is the case. So I think it's best to do that here as well.

Review of attachment 177955 [details] [review]:

Looks good

Review of attachment 178147 [details] [review]:

Code looks good. Blocked on getting icons that aren't black on the black panel.

::: js/ui/panel.js
@@ +44,3 @@
     STANDARD_TRAY_ICON_SHELL_IMPLEMENTATION['bluetooth'] = imports.ui.status.bluetooth.Indicator;
 
+const STANDARD_TRAY_INDICATOR_FACTORIES = [

Hmm, not sure I love them name "factory", but I can't think of anything better, so OK.

Err... Owen, did you check the icons provided by Jimmac (attached to the bug #616998) ? They do not look black at all to me.

And, actually, I am not sure if those icons should be packaged with g-s-d or g-s.

Created attachment 178239 [details] [review]
KeyboardStatus: handle modifier key indicators

Introduce a generic framework for on/off indicators that are shown
in the panel, next to the system status area, and use it for
showing the status of modifier keys.

Still not working because icons are not installed, by either us or gnome-settings-daemon.
Should I add all the machinery for installing themed icons?

That is a good question. Currently there are 6 icons are in g-s-d - and g-s-d is using them, as StatusIcons. Perhaps it still will be using them, in fallback mode (CMIIW).

Actually I checked the current icons shipped with g-s-d - they are also white. So I clearly do not understand, why do you get "black on black" situation? What icons do you use??

No, you misunderstood.
I thought it was a black on black because the indicator was there (it appeared from the Looking Glass), but nothing was on screen.
The real problem, though, is that it does not find the icons, either with the previous name or with the new standard one.

I see now! It cannot find them at all! Where does it look for the icons?

Something is wrong. I manually put the 3 svg files provided by jimmac to /opt/gnome/share/gnome-shell/theme - and they are not being picked up. According to strace, noone actually is trying to do anything with keyboard-caps-lock when I press Caps Lock. BUT at the same time the generic keyboard icon appears on the top panel. And the status icon (managed by g-s-d) changes the color from white to red - i.e. CapsLock events are being produced.

Also, confirming - the js handler for LED events is being called.

Owen, FYI, using short descriptions from variants is implemented in libgnomekbd. Now, the patches for xkeyboard-config are welcome;)

(In reply to comment #38)
> Something is wrong. I manually put the 3 svg files provided by jimmac to
> /opt/gnome/share/gnome-shell/theme - and they are not being picked up.
> According to strace, noone actually is trying to do anything with
> keyboard-caps-lock when I press Caps Lock. BUT at the same time the generic
> keyboard icon appears on the top panel. And the status icon (managed by g-s-d)
> changes the color from white to red - i.e. CapsLock events are being produced.

The first time it looks for an icon, it fails back to shortening the name if the icon is not found, so first 'keyboard-caps-lock-symbolic', then 'keyboard-caps-lock', then 'keyboard-caps', then 'keyboard', which is the only one it finds. It you need to force a reloading of all the icons (or just restart the shell) to see any changes, for example by changing the theme in GConf.
Also, don't forget that icons are installed into /opt/gnome/share/icons/gnome/scalable/, and you need to run gtk-update-icon-cache when done.

Created attachment 178667 [details]
strace log

Here is what I got in strace

(Giovanni, thanks again for your help on IRC).

Now, libgnomekbd installs all the necessary icons for LEDs, adds them to the icon path in the constructor. But they are still not visible.

One thing I spotted: in JS, you're creating icons for caps/num/scroll before you create GkbdConfiguration - so the icon path is not set. If I move Gkbd.Configuration.get in front of the icons creation, at least I am getting some kind of icon. But that icon is generic a keyboard icon. Is it possible that St.Icon using some cache that should be refreshed after I added more subdirs to the gtk icon path?

(In reply to comment #43)
> (Giovanni, thanks again for your help on IRC).
> 
> Now, libgnomekbd installs all the necessary icons for LEDs, adds them to the
> icon path in the constructor. But they are still not visible.
> 
> One thing I spotted: in JS, you're creating icons for caps/num/scroll before
> you create GkbdConfiguration - so the icon path is not set. If I move
> Gkbd.Configuration.get in front of the icons creation, at least I am getting
> some kind of icon. But that icon is generic a keyboard icon. Is it possible
> that St.Icon using some cache that should be refreshed after I added more
> subdirs to the gtk icon path?

Moving Gkbd.Configuration.get at the top is obviously correct, but not enough, because GtkIconTheme will prefer loading a themed icon with a fallback name (so keyboard), rather than one from the unthemed search path (keyboard-caps-lock.svg, in $(prefix)/share/libgnomekbd/icons).
A simple fix is to change the prefix to something unique (like gkbd-), so that no fallback name can possibly exist.

Shouldn't we just get icons added to gnome-icon-theme-symbolic?

> GtkIconTheme will prefer loading a themed icon with a fallback name (so
keyboard), rather than one from the unthemed search path
That's nasty. Can that preference be configured somehow?

> Shouldn't we just get icons added to gnome-icon-theme-symbolic?
Owen, Jimmac said (see bug #616998) they should be shipped with gsd - which I think is wrong. Who can authorize committing them to g-i-t-s?

FWIW, the icons now have prefix "kbdled-". Giovanni, please amend your patch:
1. Call Gkbd.Configuration.get before creating LED icons
2. Change icon names with the proper prefix

With those changes, your patch works for me - I see proper icons!

The only open issue is where to put those icons. I do not mind either way - they can stay in libgnomekbd (as fallback solution, can be overridden by a theme) - or they can move into g-i-t-s.

One more question. g-s-d in GNOME2 (and in GNOME3, fallback mode) implements GtkStatusIcon as keyboard indicator (with optional LEDs). Since gnome-shell implements some kind of notification area, these status icons appear in it. What would be the proper way for g-s-d to determine the scenario "I am running within gnome-shell, so I am not showing kbd indicators"?

Rhythmbox checks if org.gnome.Shell is known to dbus:

       bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
       if (bus != NULL) {
               GVariant *result;
               result = g_dbus_connection_call_sync (bus,
                                                     "org.freedesktop.DBus",
                                                     "/org/freedesktop/DBus",
                                                     "org.freedesktop.DBus",
                                                     "GetNameOwner",
                                                     g_variant_new ("(s)", "org.gnome.Shell"),
                                                     G_VARIANT_TYPE ("(s)"),
                                                     G_DBUS_CALL_FLAGS_NONE,
                                                     -1,
                                                     NULL,
                                                     NULL);
               if (result != NULL) {
                       rb_debug ("GNOME Shell is running");
                       plugin->priv->is_gnome_shell = TRUE;
                       g_variant_unref (result);
               } else {
                       rb_debug ("GNOME Shell isn't running");
                       plugin->priv->is_gnome_shell = FALSE;
               }
               g_object_unref (bus);
       }

Thanks, Frederic. Is this function available in some library? Or should we just copy/paste it around git?

Created attachment 178866 [details] [review]
KeyboardStatus: handle modifier key indicators

Introduce a generic framework for on/off indicators that are shown
in the panel, next to the system status area, and use it for
showing the status of modifier keys.

I am confirming, the last patch works fine. Please commit (and you can close this bug).

The only thing to discuss (offtopic here) is where to put those 3 infamous icons

Review of attachment 178866 [details] [review]:

Looks fine. While the bug isn't actually fixed:

 - We don't have the specified menu appearance
 - We don't have input method integration

it can be closed after committing this, since I don't think it's useful to have more patches and review on such a long bug.

Should a new bug be opened for the original issue?

Perhaps new bug for the input method integration would be the right thing.

About the menu appearance: the only substantial missing thing is the "Show Keyboard Layout" item, I am going to add it (regardless if this bug is closed or not).

BTW, the last menu item should be changed from "Keyboard settings" to "Localization settings" - it would be more correct (because it is invoking "g-c-c region") and match the mockup too.

Attachment 178866 [details] pushed as 20f49e8 - KeyboardStatus: handle modifier key indicators
Includes the string fix.
The missing item cannot be added because the layout is drawn with Gtk, it would require a dialog and thus a separate process.

Giovanni, if I add the function that pops up the dialog (to Gkbd.Configuration) - would JS be able to call it?

(In reply to comment #58)
> Giovanni, if I add the function that pops up the dialog (to Gkbd.Configuration)
> - would JS be able to call it?

No. Although the function could be called from JS, the problem is that it cannot be called from GNOME Shell (or any other wm for what it's worth). Basically a window manager can only create overwrite-redirect windows, so a normal dialog must be created from another process.

(As Owen has proposed in comment 53, further discussion should happen in a new bug report)

Right. Created bug #640226

bug #641531 for IBus.

For some reason, the CAPS/NUM/SCROLL lock indicators were removed from the code. Could anybody explain - why?

(In reply to comment #62)
> For some reason, the CAPS/NUM/SCROLL lock indicators were removed from the
> code. Could anybody explain - why?

They weren't part of the design. They weren't "removed" as much as never added.

Bastien, what is provided by "the design" as an indication for the lock indicators? IIRC WIN 7 (or OEM custom versions?) have that thing. Is it left for extensions? It is really nice thing to have, these days many laptops and wireless kbds do not have HW LEDs.

Sergey, ask the designers about that. Commenting on a closed bug isn't going to bring much results...