GNOME Bugzilla – Bug 779413
Translated X-Geoclue-Reason string not used in a dialog window
Last modified: 2018-01-08 23:30:02 UTC
Created attachment 346963 [details] gnome-clocks showing X-Geoclue-Reason in English When opening gnome-clocks (or gnome-weather [separate bug report]) for the first time, GNOME shows a dialog that asks for permission to access location (since commit 346f1e8e0f6143018dd7d4970fd2001fa8516ce7). This dialog should use X-Geoclue-Reason string that belongs to https://git.gnome.org/browse/gnome-clocks/tree/data/org.gnome.clocks.desktop.in "X-Geoclue-Reason=Allows world clocks to be displayed for your time zone." This string has been in .pot file since 3.19 and has been translated in Finnish (since commit b166667bed53d52eb9610261e301714de905ee9e) before the release of 3.20 so it should be used at least in 3.22. However, the dialog still uses English string instead of translated string. This bug affects 3.22 on fully up-to-date Fedora 25, and as far as I can remember, both 3.20 (Fedora 24) and 3.24 (Fedora 26) as well. [name@kabylake ~]$ dnf info gnome-clocks Name : gnome-clocks Arkkitehtuu : x86_64 Epoch : 0 Versio : 3.22.1 Release : 1.fc25 Koko : 1.3 M Repo : @System From repo : anaconda Summary : Clock application designed for GNOME 3 URL : https://wiki.gnome.org/Apps/Clocks License : GPLv2+ Description : Clock application designed for GNOME 3 [name@kabylake ~]$ cat /etc/redhat-release Fedora release 25 (Twenty Five)
I can confirm this bug (and bug #779415). I believe gnome-shell is responsible for displaying this string.
*** Bug 779415 has been marked as a duplicate of this bug. ***
(In reply to Piotr Drąg from comment #1) > I believe gnome-shell is responsible for displaying this string. Yes, but all we have for looking up a custom string field from a .desktop file is g_desktop_app_info_get_string(), which isn't localized. We are not going to guess random apps' translation domains to look up the translations, so we'd need API in GIO for this first (basically a small wrapper around g_key_file_get_locale_string()).
(In reply to Florian Müllner from comment #3) > (In reply to Piotr Drąg from comment #1) > > I believe gnome-shell is responsible for displaying this string. > > Yes, but all we have for looking up a custom string field from a .desktop > file is g_desktop_app_info_get_string(), which isn't localized. We are not > going to guess random apps' translation domains to look up the translations, > so we'd need API in GIO for this first (basically a small wrapper around > g_key_file_get_locale_string()). Patch welcome: it seems like a reasonable API addition to GDesktopAppInfo.
Created attachment 366451 [details] [review] gdesktopappinfo: Add g_desktop_app_info_get_locale_string() Custom desktop file fields may be translated, but there is currently no non-hacky way to look up the localized value; fill get gap with a small wrapper around g_key_file_get_locale_string().
Review of attachment 366451 [details] [review]: Could you also add a test for this in gio/tests/desktop-app-info.c please? ::: gio/gdesktopappinfo.c @@ +4418,3 @@ +/** + * g_desktop_app_info_get_string: get_locale_string @@ +4422,3 @@ + * @key: the key to look up + * + * Looks up a localized string value in the keyfile backing @info. Add something like ‘and translates it to the current locale’? Actually, I think it would be better to explicitly expose a `locale` argument, like g_key_file_get_locale_string() does. Passing NULL to it would use the locale from the environment. That reduces our reliance on global state a bit. @@ +4426,3 @@ + * The @key is looked up in the "Desktop Entry" group. + * + * Returns: a newly allocated string, or %NULL if the key (nullable) @@ +4435,3 @@ + const char *key) +{ + g_return_val_if_fail (G_IS_DESKTOP_APP_INFO (info), NULL); Plus g_return_val_if_fail (key != NULL && *key != '\0', NULL);
(In reply to Philip Withnall from comment #6) > Actually, I think it would be better to explicitly expose a `locale` > argument, like g_key_file_get_locale_string() does. Passing NULL to it would > use the locale from the environment. That reduces our reliance on global > state a bit. In order for this to work (for any locale other than the current one), we need to change the flags passed to the various g_key_file_load_*() calls to include G_KEY_FILE_KEEP_TRANSLATIONS. Do you want me to make that change, or do you prefer the fixed-locale version under those circumstances?
(In reply to Florian Müllner from comment #7) > (In reply to Philip Withnall from comment #6) > > Actually, I think it would be better to explicitly expose a `locale` > > argument, like g_key_file_get_locale_string() does. Passing NULL to it would > > use the locale from the environment. That reduces our reliance on global > > state a bit. > > In order for this to work (for any locale other than the current one), we > need to change the flags passed to the various g_key_file_load_*() calls to > include G_KEY_FILE_KEEP_TRANSLATIONS. > > Do you want me to make that change, or do you prefer the fixed-locale > version under those circumstances? Hmm, I forgot about the need for that. That’s going to add a fair amount of memory overhead to each GDesktopAppInfo, for minimal benefit. It’s probably better to add a KEEP_TRANSLATIONS flag for when loading a GDesktopAppInfo, or to add a g_desktop_app_info_get_key_file() API in future if someone needs it. Let’s not do that now though. Scrap the `locale` parameter then. Thanks for querying!
I’ve also filed https://bugzilla.gnome.org/show_bug.cgi?id=792324 about documenting the need for KEEP_TRANSLATIONS when using these GKeyFile APIs.
(In reply to Philip Withnall from comment #8) > Hmm, I forgot about the need for that. So did I, but you made me add a test case that was failing ;-)
Created attachment 366484 [details] [review] gdesktopappinfo: Add g_desktop_app_info_get_locale_string() Updated documentation, added test case.
Review of attachment 366484 [details] [review]: Looks good to push with this change: ::: gio/tests/desktop-app-info.c @@ +377,3 @@ + + g_setenv ("LANGUAGE", lang, TRUE); + setlocale (LC_ALL, ""); Would be good to also test the fallback: g_setenv ("LANGUAGE", "sv_SV.UTF8", TRUE); setlocale (LC_ALL, ""); s = g_desktop_app_info_get_locale_string (appinfo, "X-JunkFood"); g_assert_cmpstr (s, ==, "Burger"); /* fallback */ g_free (s); before resetting the LANGUAGE to `lang`.
Created attachment 366496 [details] [review] gdesktopappinfo: Add g_desktop_app_info_get_locale_string()
Comment on attachment 366496 [details] [review] gdesktopappinfo: Add g_desktop_app_info_get_locale_string() Attachment 366496 [details] pushed as a55bfee - gdesktopappinfo: Add g_desktop_app_info_get_locale_string()
Created attachment 366498 [details] [review] location: Use translated reason string Gio now has API to look up localized strings from desktop files, so we can display a correctly translated reason where available. This should fix the original issue once we get a gobject-introspection release with the new API.
(In reply to Florian Müllner from comment #15) > This should fix the original issue once we get a gobject-introspection > release with the new API. Which just happened with 1.55.1.
Attachment 366498 [details] pushed as 723c49a - location: Use translated reason string