Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#11337 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: i18n Version:
Severity: normal Keywords: language, preferences
Cc: Branch:
Release Notes:

The Language preferences panel is shown to users with TRAC_ADMIN even if Babel is not installed. A hint is shown on the Basic Settings admin page when the message catalogs aren't compiled. The Language select on the Language preferences and Basic Settings panel are disabled when Babel is not installed or the message catalogs are not compiled.

API Changes:

When compiled catalogs are not available get_negotiated_locale() returns en_US instead of get_available_locales() returning en_US.

Internal Changes:

Description

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

    diff --git a/trac/prefs/web_ui.py b/trac/prefs/web_ui.py
    index 3a69422..d09419a 100644
    a b class PreferencesModule(Component):  
    9595        yield ('datetime', _('Date & Time'))
    9696        yield ('keybindings', _('Keyboard Shortcuts'))
    9797        yield ('userinterface', _('User Interface'))
    98         if Locale:
     98        if Locale or 'TRAC_ADMIN' in req.perm:
    9999            yield ('language', _('Language'))
    100100        if not req.authname or req.authname == 'anonymous':
    101101            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.

Attachments (6)

LanguagePrefencesPage.png (34.6 KB ) - added by Ryan J Ollos 11 years ago.
BasicSettingsBabelNotInstalled.png (16.5 KB ) - added by Ryan J Ollos 11 years ago.
LanguagePreferencesBabelNotInstalledAdmin.png (16.5 KB ) - added by Ryan J Ollos 11 years ago.
BasicSettingsMessageCatalogsNotCompiled.png (14.2 KB ) - added by Ryan J Ollos 11 years ago.
LanguagePreferencesBabelInstalledMessageCatalogsNotCompiled.png (14.8 KB ) - added by Ryan J Ollos 11 years ago.
LanguagePreferencesBabelInstalledMessageCatalogsNotCompiledAdmin.png (14.4 KB ) - added by Ryan J Ollos 11 years ago.

Download all attachments as: .zip

Change History (24)

by Ryan J Ollos, 11 years ago

Attachment: LanguagePrefencesPage.png added

comment:1 by Ryan J Ollos, 11 years ago

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:

in reply to:  1 comment:2 by Ryan J Ollos, 11 years ago

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.

comment:3 by Jun Omae, 11 years ago

That makes sense.

in reply to:  3 ; comment:4 by Ryan J Ollos, 11 years ago

Status: newassigned

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.

in reply to:  4 comment:5 by Jun Omae, 11 years ago

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.

comment:6 by Jun Omae, 11 years ago

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.

comment:7 by Jun Omae, 11 years ago

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.

comment:8 by Ryan J Ollos, 11 years ago

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.

in reply to:  7 comment:10 by Jun Omae, 11 years ago

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.

by Ryan J Ollos, 11 years ago

comment:11 by Ryan J Ollos, 11 years ago

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

comment:12 by Ryan J Ollos, 11 years ago

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

    diff --git a/trac/admin/web_ui.py b/trac/admin/web_ui.py
    index b07ff8a..5ca2a59 100644
    a b class BasicsAdminPanel(Component):  
    228228            self.config.set('trac', 'default_timezone', default_timezone)
    229229
    230230            default_language = req.args.get('default_language')
    231             if default_language not in locales:
     231            if Locale.parse(default_language) not in locales:
    232232                default_language = ''
    233233            self.config.set('trac', 'default_language', default_language)

in reply to:  12 ; comment:13 by Jun Omae, 11 years ago

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

in reply to:  13 ; comment:14 by Jun Omae, 11 years ago

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].

comment:15 by Ryan J Ollos, 11 years ago

API Changes: modified (diff)
Release Notes: modified (diff)

in reply to:  14 comment:16 by Jun Omae, 11 years ago

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].

comment:17 by Ryan J Ollos, 11 years ago

API Changes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

comment:18 by Ryan J Ollos, 10 years ago

Keywords: language preferences → language, preferences

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.