GNOME Bugzilla – Bug 710412
bogus locale encoding appended
Last modified: 2014-10-29 14:32:40 UTC
Hi. Is there any reason to a codeset specifier to locale names? The encoding part of a locale is optional and locale normalisation is already done by the gnome-desktop library, and adding something here gets in the way of that. On OpenBSD this leads to warnings like this when entering the Users capplet: (gnome-control-center:30413): GnomeDesktop-WARNING **: locale 'en_US.UTF-8.utf8' isn't valid Is there any reason to do what is currently done; is it needed on Linux for some reason? This patch fixes the issue for me. I did not git-format it because I want to make sure I am not missing something obvious here and would love to get some feedback on that idea... Thanks for looking. --- panels/common/cc-common-language.c.orig Thu Oct 17 18:09:54 2013 +++ panels/common/cc-common-language.c Thu Oct 17 18:24:50 2013 @@ -601,11 +601,7 @@ insert_language (GHashTable *ht, g_debug ("We have translations for %s", lang); - if (g_str_has_suffix (lang, ".utf8")) - key = g_strdup (lang); - else - key = g_strdup_printf ("%s.utf8", lang); - + key = g_strdup (lang); label_own_lang = gnome_get_language_from_locale (key, key); label_current_lang = gnome_get_language_from_locale (key, NULL); label_untranslated = gnome_get_language_from_locale (key, "C");
Created attachment 258173 [details] [review] do not add a codeset specifier Any thoughts on that? git-format patch added.
See bug 709272
(In reply to comment #2) > See bug 709272 Thanks Rui, I missed it somehow. *** This bug has been marked as a duplicate of bug 709272 ***
Hmm actually, while related, the issue is different here, reopening.
So now that https://bugzilla.gnome.org/show_bug.cgi?id=709272 has been committed, wouldn't it make sense to use this patch as well? Thanks.
It would be nice to have some inputs guys... ;-)
Review of attachment 258173 [details] [review]: This unfortunately doesn't work. We really need the suffix here because the keys on this hash table need to be compared with the return of gnome_get_all_locales() which always returns with the suffix. And we need the suffix _there_ because we use those locale strings to set your session and/or system locale and you definitely don't want a non-UTF-8 locale if you can avoid it. I'm going to attach a couple of patches which make UTF-8 be the preferred suffix instead, seeing as that seems to work both on OpenBSD and GNU libc.
Created attachment 266811 [details] [review] languages: Use a more broadly compatible locale codeset suffix At least OpenBSD's libc doesn't accept 'utf8' as a locale codeset suffix but does accept 'UTF-8'. Since GNU libc accepts both suffixes let's use the one which works on a broader set of systems. -- The normalization done here isn't of course what we were doing but do we care about any other codesets?
Created attachment 266812 [details] [review] Use 'UTF-8' instead of 'utf8' as locale codeset suffix This makes us work on OpenBSD's libc. GNU libc accepts both suffixes.
Review of attachment 266812 [details] [review]: ::: panels/common/cc-common-language.c @@ +752,3 @@ + + if (!codeset || !g_str_equal (codeset, "UTF-8")) + g_warning ("Current user locale codeset isn't UTF-8"); That shouldn't be a warning. We still support other locales, and if I want to run my system in fr_FR.ISO-8859-15 it should be able to.
Review of attachment 266811 [details] [review]: ::: libgnome-desktop/gnome-languages.c @@ -112,1 @@ - return normalized_codeset; This will stop normalising things like ISO-88590-15 as a codeset, is that expected?
(In reply to comment #10) > Review of attachment 266812 [details] [review]: > > ::: panels/common/cc-common-language.c > @@ +752,3 @@ > + > + if (!codeset || !g_str_equal (codeset, "UTF-8")) > + g_warning ("Current user locale codeset isn't UTF-8"); > > That shouldn't be a warning. Sure, I'll remove that. > We still support other locales, and if I want to > run my system in fr_FR.ISO-8859-15 it should be able to. I don't know all the ramifications of this, I admit. I guess most apps will run fine, sure. My big concern is what we present to users. I definitely don't think presenting users with a list of languages like: ... French (France) French (France) [ISO-8859-15] ... is something we want. Which means we have to standardize on a codeset. In fact, gnome-languages.c has always enforced this[1], i.e. when you query the list of locales with gnome_get_all_locales() it will *only* return UTF-8 locales. So yeah, in practice, using GNOME UI, you can't set a non-UTF-8 locale and I think that's perfectly fine and in fact the only sane thing to do. [1] see add_locale(), it takes a boolean utf8_only argument which is always TRUE[2]. It was like that already when I moved the code to gnome-desktop. [2] yeah, I think I should clean that up and just remove that argument...
(In reply to comment #11) > Review of attachment 266811 [details] [review]: > > ::: libgnome-desktop/gnome-languages.c > @@ -112,1 @@ > - return normalized_codeset; > > This will stop normalising things like ISO-88590-15 as a codeset, is that > expected? Yes it's expected. With the way things work currently I can't see it breaking anything.
(In reply to comment #9) > Created an attachment (id=266812) [details] [review] > Use 'UTF-8' instead of 'utf8' as locale codeset suffix > > This makes us work on OpenBSD's libc. GNU libc accepts both suffixes. Hi Rui. When running with only the gnome-cc patch, all is fine. If I add up the gnome-desktop patch I am getting this: (gnome-control-center:10930): GnomeDesktop-WARNING **: locale 'Pig.ISO8859-1.UTF-8' isn't valid And gnome-cc core dumps.
+ Trace 233063
Hi guys. I think you should go ahead with these patches if you think they are fine for Linux as well. The issue I mentioned is OpenBSD specific and will eventually be fixed there. Thanks.
Created attachment 274127 [details] [review] languages: Use a more broadly compatible locale codeset suffix v2 Actually this new patch for gnome-desktop fixes *all* issues I've been seeing on OpenBSD (along with your gnome-cc patch). I regened the patch using git-format so it comes with my name on it -- if this ever goes in, please amend it to drop me and take credit for it as well as Stefan Sperling (our locale guy) who came up with the modified patch. I hope this is OK for you guys, it would bring me one step closer to have a working jhbuild run on OpenBSD... Thank you.
Created attachment 274269 [details] [review] cc-common-language.c: Remove a bunch of unused code -- Lots of dead code in this file. This patch isn't stricly needed.
Created attachment 274270 [details] [review] cc-common-language.c: Don't check if we have translations here The list of locales returned from gnome_get_all_locales() already filters out the locales for which we don't have translations so this is a redundant check since users of get_initial_languages() only show a language if it exists in gnome_get_all_locales() . In fact, this code stopped working correctly since we started passing locales including the codeset suffix to insert_language() because the translation directories usually don't include the suffix and we weren't stripping the suffix here. -- This one is needed if we go forward with attachment 266812 [details] [review] .
Review of attachment 274127 [details] [review]: ::: libgnome-desktop/gnome-languages.c @@ +401,3 @@ name = g_strdup (language_name); } else if (utf8_only) { + name = g_strdup_printf ("%s.UTF-8", language_name); this is leaked. @@ +415,2 @@ } + return FALSE; this should be: else { return FALSE; }
Created attachment 274272 [details] [review] languages: Use a more broadly compatible locale codeset suffix -- Antoine can you test this one? Your patch had some thinkos as I pointed out.
> Antoine can you test this one? Your patch had some thinkos as I > pointed out. Hi Rui. This one works just fine, thanks :-) Regarding your gnome-cc patches, you left out the chunk in panels/region/cc-input-chooser.c, was it on purpose?
(In reply to comment #21) > Regarding your gnome-cc patches, you left out the chunk in > panels/region/cc-input-chooser.c, was it on purpose? You mean - gchar *locale = g_strdup_printf ("%s_%s.utf8", lang_code, country_code); + gchar *locale = g_strdup_printf ("%s_%s.UTF-8", lang_code, country_code); ? Yes this is on purpose. We're changing everything to use UTF-8, right?
> You mean > > - gchar *locale = g_strdup_printf ("%s_%s.utf8", lang_code, > country_code); > + gchar *locale = g_strdup_printf ("%s_%s.UTF-8", lang_code, > country_code); > > ? Yes this is on purpose. We're changing everything to use UTF-8, right? Indeed. I was just confused as to why we were keeping utf8 instead of moving to UTF-8.
Hi Rui. Any hold up on these patches? Thanks :-)
Another ping... it'd be really nice if we could get these in. Or at least know if there is a problem with them that we need to address. I kept these patches updated for 3.14 at: http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/ports/x11/gnome/controlcenter/patches/patch-panels_common_cc-common-language_c?rev=1.6 http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/ports/x11/gnome/controlcenter/patches/patch-panels_region_cc-input-chooser_c?rev=1.2 http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/ports/x11/gnome/desktop/patches/patch-libgnome-desktop_gnome-languages_c?rev=1.8 Thanks.
Rui I'd really appreciate if you could comment on this. Thanks in advance.
Review of attachment 274272 [details] [review]: Looks good.
Review of attachment 274270 [details] [review]: Sure.
Review of attachment 274269 [details] [review]: Reasoning as to why it's unused would be helpful (is it dead code? when did we stop using it?)
(In reply to comment #29) > Review of attachment 274269 [details] [review]: > > Reasoning as to why it's unused would be helpful (is it dead code? when did we > stop using it?) We stopped using them with the not-so-new-anymore design in https://git.gnome.org/browse/gnome-control-center/commit/?id=35d920f1b887fb0f1df243e4d80ef8165a4f772e . I'll add the reference to the commit message.
Comment on attachment 274272 [details] [review] languages: Use a more broadly compatible locale codeset suffix Attachment 274272 [details] pushed as 093e0a5 - languages: Use a more broadly compatible locale codeset suffix
All pushed to the respective master branches. Antoine, sorry for taking so long with this. You're of course free to use the patches in 3.14 for OpenBSD if they work fine for yout but I didn't want to push them upstream for that branch for fear of regressing something. Attachment 266812 [details] pushed as 880f9f1 - Use 'UTF-8' instead of 'utf8' as locale codeset suffix Attachment 274269 [details] pushed as c244568 - cc-common-language.c: Remove a bunch of unused code Attachment 274270 [details] pushed as ed273aa - cc-common-language.c: Don't check if we have translations here
> Antoine, sorry for taking so long with this. You're of course free to No problem, I was already using them in 3.12 and then 3.14. > use the patches in 3.14 for OpenBSD if they work fine for yout but I > didn't want to push them upstream for that branch for fear of > regressing something. Ok. Anyway, thanks to you and Bastien for looking at them. Cheers!