Language preferences panel not shown when user has TRAC_ADMIN and Babel is not installed

[Ryan J Ollos]

There is text​ on the preferences panel as well as a permissions check that suggests the panel should be shown to users with TRAC_ADMIN when Babel is not installed. However, the panel is not shown, and the following patch is needed:

trac/prefs/web_ui.py

@@ -95 +95 @@
         yield ('datetime', _('Date & Time'))
         yield ('keybindings', _('Keyboard Shortcuts'))
         yield ('userinterface', _('User Interface'))
-        if Locale:
+        if Locale or 'TRAC_ADMIN' in req.perm:
             yield ('language', _('Language'))
         if not req.authname or req.authname == 'anonymous':
             yield ('advanced', _('Advanced'))

A functional test should be added as well.

It may also be possible to make the Note more easily seen when languages are not available:

• Disable the select box.
• Hide the first two lines of text and only show the Note: Translations are currently unavailable.

There is also text​ that would be displayed for all users when languages are not available, however the preferences panel will not be shown when locale is None​, and locale is None when Babel is not installed​, therefore I can't see a case where the text will ever be visible.

[Ryan J Ollos]

There is also a minor bit of refactoring that seems appropriate. There are two places where the import from babel.core import Locale is used to determine if Babel is installed, whereas the rest of the codebase uses from trac.util.translation import has_babel:

• tags/trac-1.0.1/trac/prefs/web_ui.py@:20-23#L1​
• tags/trac-1.0.1/trac/admin/web_ui.py@:25-28#L1​

[Ryan J Ollos]

Replying to rjollos:

There is also a minor bit of refactoring that seems appropriate.

Looking at it more closely, the Locale object is needed since Locale.parse is used. The return value of get_available_locales is only used in two place, one of which the only the truthfulness of the return value is used (trac.web.chrome), and in the other two places the list is immediately parsed to Locale objects: locales = [Locale.parse(locale) for locale in get_available_locales()]. So presumably get_available_locales could just return Locale objects, but I don't know the API well enough to say for sure, so I'll just leave it for now.

[Jun Omae]

That makes sense.

[Ryan J Ollos]

Replying to jomae:

That makes sense.

Where you referring to the changes suggested in comment:description, or the changes suggested in comment:1 / comment:2?

Proposed changes as described in comment:description can be found in log:rjollos.git:t11337.

[Jun Omae]

Replying to rjollos:

Where you referring to the changes suggested in comment:description, or the changes suggested in comment:1 / comment:2?

Your patch in the description looks good to me.

Also, I don't think that get_available_locales() should a list of instances of Locale.

1. It is used in get_negotiated_locale at trac/util/translation.py. Locale.negotiate() accepts locale ids as list of str instances.
2. Locale.parse() is slow for non-cached locale. The get_negotiated_locale method is called for every request. Therefore, the first request will be very slow.
3. There is a similar babel.localedata.list method. The method returns a list of locale ids as str. (instead, get_known_locales for Babel 1.0+).
4. ​th:MailPlugin uses the method, see ​th:source:mailplugin/0.12/xmail/XMailEMailModule.py.

I'll try log:rjollos.git:t11337 branch later.

[Jun Omae]

I tried it. In [56225b7f/rjollos.git], "Please contact your …" in prefs/language page never be shown. I suppose that text is intended for use when Babel is installed but no compiled catalogs (install Babel after installing Trac without Babel). But it doesn't work after [11690] because get_available_locales() returns en_US even if no compiled catalogs.

[Jun Omae]

Proposed fix can be found in [5bd1a1b0/jomae.git], showing "Please contact your …" message for non-admin access when Babel is installed and no compiled catalogs.

[Ryan J Ollos]

Thanks, your changes look good. I wasn't sure how to deal with that case, but I've been wanting to find some way to give a better indicator to the user when Babel is installed but translations are not available.

I'm thinking now about whether we should provided a hint to the administrator on the Language preferences page and Basic Settings page when Babel is installed but the catalogs aren't compiled. I'll investigate that.

[Jun Omae]

See #9439 and [10244,10156,10154,10134].

[Jun Omae]

Replying to jomae:

Proposed fix can be found in [5bd1a1b0/jomae.git], ….

I reworked the fix. New proposed fix for [56225b7f/rjollos.git] and [11690] can be found in log:jomae.git:t11337.2.1. I think the it is simple rather than log:jomae.git:t11337.2.

[Ryan J Ollos]

A few additional changes on top of log:jomae.git:t11337.2.1 can be found in log:rjollos.git:t11337. The cases are shown in the screen captures that follow.

Basic Settings: Babel not installed

Basic Settings: Babel installed, Message catalogs not compiled

Language Preferences: Babel not installed

Language preferences panel not displayed

Language Preferences: Babel not installed, has TRAC_ADMIN

Language Preferences: Babel installed, Message catalogs not compiled

Language Preferences: Babel installed, Message catalogs not compiled, has TRAC_ADMIN

[Ryan J Ollos]

On current 1.0-stable, I'm unable to change the language through the Basic Settings page. I can change the language through the Language preferences page.

For example, when changing to español, the local variables are:

default_language =  es
locales =  [Locale('he'), Locale('gl'), Locale('fa'), Locale('nb'), Locale('en', territory='GB'), Locale('fi'), Locale('it'), Locale('es', territory='MX'), Locale('ja'), Locale('vi'), Locale('en', territory='US'), Locale('et'), Locale('ca'), Locale('hu'), Locale('cs'), Locale('tr'), Locale('fr'), Locale('hy'), Locale('sv'), Locale('eo'), Locale('pt', territory='BR'), Locale('zh', territory='CN', script='Hans'), Locale('de'), Locale('pt'), Locale('ro'), Locale('ko'), Locale('ru'), Locale('el'), Locale('es', territory='AR'), Locale('sl'), Locale('zh', territory='TW', script='Hant'), Locale('pl'), Locale('nl'), Locale('da'), Locale('es'), Locale('uk')]

so this conditional​ evaluates to True, and the default_language is set to an empty string.

The following patch seems to be needed:

trac/admin/web_ui.py

@@ -228 +228 @@
             self.config.set('trac', 'default_timezone', default_timezone)
 
             default_language = req.args.get('default_language')
-            if default_language not in locales:
+            if Locale.parse(default_language) not in locales:
                 default_language = ''
             self.config.set('trac', 'default_language', default_language)

[Jun Omae]

Replying to rjollos:

On current 1.0-stable, I'm unable to change the language through the Basic Settings page. I can change the language through the Language preferences page.

That's an issue with Babel 1.x. I'll fix at #11258.

>>> from babel import __version__
>>> __version__
'0.9.6'
>>> from babel.core import Locale
>>> 'es' not in [Locale.parse('es'), Locale.parse('es_MX')]
False
>>> 'es' == Locale.parse('es')
True
>>> 'en_US' == Locale.parse('en_US')
True
>>>

>>> from babel import __version__
>>> __version__
'1.3'
>>> from babel.core import Locale
>>> 'es' not in [Locale.parse('es'), Locale.parse('es_MX')]
True
>>> 'es' == Locale.parse('es')
False
>>> 'en_US' == Locale.parse('en_US')
False

[Jun Omae]

Replying to jomae:

Replying to rjollos:

On current 1.0-stable, I'm unable to change the language through the Basic Settings page. I can change the language through the Language preferences page.

That's an issue with Babel 1.x. I'll fix at #11258.

Proposed fix for it can be found in [ff5178d3/jomae.git].

[Jun Omae]

On current 1.0-stable, I'm unable to change the language through the Basic Settings page. I can change the language through the Language preferences page.

That's an issue with Babel 1.x. I'll fix at #11258.

Proposed fix for it can be found in [ff5178d3/jomae.git].

Fixed in [12205].

[Ryan J Ollos]

Committed to 1.0-stable in [12211:12212] and merged to trunk in [12213]. Thanks Jun!