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 695049 - Can't select m17n input methods for languages that don't have a locale
Can't select m17n input methods for languages that don't have a locale
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Region & Language
git master
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-03 11:18 UTC by Neil Roberts
Modified: 2013-03-11 13:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
region: Factor out adding ibus engine widgets code (2.17 KB, patch)
2013-03-04 14:46 UTC, Rui Matos
none Details | Review
region: Always add ibus engine widgets regardless of locale info (2.13 KB, patch)
2013-03-04 14:47 UTC, Rui Matos
none Details | Review
region: Factor out adding source widgets code (3.30 KB, patch)
2013-03-08 20:59 UTC, Rui Matos
committed Details | Review
region: Always add ibus engine widgets regardless of locale info (1.34 KB, patch)
2013-03-08 20:59 UTC, Rui Matos
committed Details | Review

Description Neil Roberts 2013-03-03 11:18:55 UTC
IBus/m17n has some useful input methods for Esperanto. However, it's not possible to directly select these in git master of the regions panel because as far as I can tell it now limits the methods to the ones whose language has a corresponding locale. glibc has historically blocked adding an Esperanto locale with no explanation. Although some distributions manually patch it in, notably Fedora doesn't. In the old control center it was possible to select one of the Esperanto input methods but presumably I won't be able to in the next Gnome. Is there anything we can do about this?

http://sourceware.org/bugzilla/show_bug.cgi?id=2135
https://bugzilla.redhat.com/show_bug.cgi?id=684474
Comment 1 Neil Roberts 2013-03-03 17:54:50 UTC
Actually it looks like glibc has had a change of heart about accepting locales for artificial languages since the maintainership changed. I've proposed a patch to add an Esperanto locale so if that gets accepted then this won't be such a big deal at least for Esperanto.

http://sourceware.org/bugzilla/show_bug.cgi?id=14943
Comment 2 Rui Matos 2013-03-03 21:55:25 UTC
There's an "Other" last row in the chooser which has everything that doesn't have locale data. If you just search for esperanto (after clicking the "…" row) you should also see those entries there.
Comment 3 Neil Roberts 2013-03-04 14:20:37 UTC
I don't think that works quite right. If I press the ‘…’ button and search for Esperanto I only get the four keyboard layouts, not the input methods provided by m17n. If I manually generate my own Esperanto locale and run it again then I additionally get the 6 extra input methods from m17n.

It looks like the input methods get filtered out by this bit of code:

http://git.gnome.org/browse/gnome-control-center/tree/panels/region/cc-input-chooser.c?id=730dbfd75ab04a4b47cb38c805fda305253b2c36#n843

The input method has a language code (eo) so it hits the second branch of the if-statement (if (lang_code != NULL)…). That code then tries to find a locale for that language code, but of course there isn't one so it doesn't call add_widgets_to_table. Maybe it should be changed so that it will hit the third part of the if statement (the bit about ‘add it to the “other” locale’) in that case.
Comment 4 Rui Matos 2013-03-04 14:46:24 UTC
Created attachment 237992 [details] [review]
region: Factor out adding ibus engine widgets code
Comment 5 Rui Matos 2013-03-04 14:47:42 UTC
Created attachment 237993 [details] [review]
region: Always add ibus engine widgets regardless of locale info

This makes sure any engine we don't have locale info for ends up under
the Other row.
--

(In reply to comment #3)
> It looks like the input methods get filtered out by this bit of code:
>
> http://git.gnome.org/browse/gnome-control-center/tree/panels/region/cc-input-chooser.c?id=730dbfd75ab04a4b47cb38c805fda305253b2c36#n843
>
> The input method has a language code (eo) so it hits the second branch of the
> if-statement (if (lang_code != NULL)…). That code then tries to find a locale
> for that language code, but of course there isn't one so it doesn't call
> add_widgets_to_table. Maybe it should be changed so that it will hit the third
> part of the if statement (the bit about ‘add it to the “other” locale’) in that
> case.

Ups, you're right. Thanks for pointing it out.
Comment 6 Neil Roberts 2013-03-04 18:33:42 UTC
Those patches work for me. Thanks!
Comment 7 Rui Matos 2013-03-08 20:59:31 UTC
Created attachment 238410 [details] [review]
region: Factor out adding source widgets code

--

Found a forgotten hunk to consolidate here.
Comment 8 Rui Matos 2013-03-08 20:59:50 UTC
Created attachment 238411 [details] [review]
region: Always add ibus engine widgets regardless of locale info

--

rebased
Comment 9 Matthias Clasen 2013-03-09 22:28:13 UTC
Should we consider this for 3.8 ?
Comment 10 Rui Matos 2013-03-09 22:51:03 UTC
(In reply to comment #9)
> Should we consider this for 3.8 ?

Yes - it's a bug fix without UI changes (well, shows entries that would otherwise be hidden but among hundreds of others...)
Comment 11 Mike FABIAN 2013-03-10 11:33:05 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Should we consider this for 3.8 ?
> 
> Yes - it's a bug fix without UI changes (well, shows entries that would
> otherwise be hidden but among hundreds of others...)

And it does not only affect Esperanto but also general purpose input methods
like latn-post and latn-pre, so it is really nice to have.
Comment 12 Bastien Nocera 2013-03-11 10:07:35 UTC
Review of attachment 238410 [details] [review]:

::: panels/region/cc-input-chooser.c
@@ +775,3 @@
+            const gchar *id)
+{
+  GList tmp = { 0 };

Why set the GList's data pointer here.

@@ +776,3 @@
+{
+  GList tmp = { 0 };
+  tmp.data = (gpointer) id;

and then here?

Should prev/next be reset as well?
Comment 13 Bastien Nocera 2013-03-11 10:08:33 UTC
Review of attachment 238411 [details] [review]:

Fine.
Comment 14 Rui Matos 2013-03-11 11:07:21 UTC
(In reply to comment #12)
> ::: panels/region/cc-input-chooser.c
> @@ +775,3 @@
> +            const gchar *id)
> +{
> +  GList tmp = { 0 };
> 
> Why set the GList's data pointer here.
> 
> @@ +776,3 @@
> +{
> +  GList tmp = { 0 };
> +  tmp.data = (gpointer) id;
> 
> and then here?
> 
> Should prev/next be reset as well?

It's just because add_widgets_to_table() takes a GList with ids to create widgets for. Here I just want to add a single widget so yes, prev/next should be NULL.
Comment 15 Neil Roberts 2013-03-11 12:21:16 UTC
> +  GList tmp = { 0 };
>
> Should prev/next be reset as well?

Initialising any of the members of a stack-allocated struct will also reset any of the unspecified members to zero according to the C spec, so this effectively does reset prev/next too.

“If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration.”
Comment 16 Rui Matos 2013-03-11 13:01:10 UTC
Attachment 238410 [details] pushed as 3dc1968 - region: Factor out adding source widgets code
Attachment 238411 [details] pushed as ce5d794 - region: Always add ibus engine widgets regardless of locale info