Opened 7 years ago

Last modified 12 months ago

#28567 new Bug

Incompatibility between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False.

Reported by: George Tantiras Owned by:
Component: Internationalization Version: dev
Severity: Normal Keywords:
Cc: Claude Paroz Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Using python 3.4.

In a fresh Django 1.11 install, I configured 2 languages: English (default Language) and Greek.

When I implemented the example HTML template code to add a change language drop down in admin, everything worked as expected if prefix_default_language=True

However, the following configuration in urls.py although it works when I change from English to Greek, it will stay to the Greek page when I try to change from Greek to English:

clean = [
    url(r'^i18n/', include('django.conf.urls.i18n')),
]

admin = i18n_patterns(
    url(r'^admin/', admin.site.urls),
    prefix_default_language=False
)


urlpatterns = admin + clean

Change History (18)

comment:1 by Tim Graham, 7 years ago

Component: UncategorizedInternationalization
Type: UncategorizedBug

Can you provide a test that demonstrates the problem? Ideally, as part of Django's test suite. b8a815e9dfea89034ede7ff786551f89af84a31b may give you an idea of where to add a test.

comment:2 by George Tantiras, 7 years ago

I have written a test class in my project, for the time being. It is a challenge to integrate it to Django, so I am pasting it here until then:

from django.test import TestCase

class setlangTest(TestCase):
    def test_en2gr(self):
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'el'},
            follow=False,
            HTTP_REFERER='/admin/',
        )
        self.assertEqual(response.url, '/el/admin/', 'Did not change from English to Greek')

    def test_gr2en(self):
        # This will fail if prefix_default_language=False
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'en'},
            follow=False,
            HTTP_REFERER='/el/admin/',
        )
        self.assertEqual(response.url, '/admin/', 'Did not change from Greek to English')
Last edited 7 years ago by George Tantiras (previous) (diff)

comment:3 by George Tantiras, 7 years ago

I have created a relevant pull request which includes the above tests.

comment:4 by Tomer Chachamu, 7 years ago

Owner: changed from nobody to Tomer Chachamu
Status: newassigned

comment:5 by Tomer Chachamu, 7 years ago

Needs documentation: set
Owner: Tomer Chachamu removed
Status: assignednew
Summary: Set Language Redirect View -> 404 if prefix_default_language=FalseUnclear documentation for 'next' parameter of set_language view
Triage Stage: UnreviewedAccepted

Thanks for that test, it really clarified it.

In the HTML in the documentation you mentioned, it mentions the redirect_to context variable, but doesn't really explain what to set it to. In your case, it is empty, so the set_language view has started looking at the HTTP_REFERRER as shown in your tests.

You need to set redirect_to to the current page URL, without the language prefix. For example, you could add this context processor to your settings:

from django.urls import translate_url

def redirect_path_context_processor(request):
    return {'language_select_redirect_to': translate_url(request.path, settings.LANGUAGE_CODE)}

The set_language view will then translate the URL again to the user's selected language.

Also, if you *do* use prefix_default_language=True, then the URL for the set_language view should also be prefixed. I don't think this is documented anywhere.

comment:6 by Tomer Chachamu, 7 years ago

Here are some passing tests showing how the view will behave when you have set next correctly.

@override_settings(
    USE_I18N=True,
    LANGUAGES=[
        ('en', 'English'),
        ('fr', 'French'),
        ('de', 'German'),
    ],
    MIDDLEWARE=[
        'django.contrib.sessions.middleware.SessionMiddleware',
        'django.contrib.auth.middleware.AuthenticationMiddleware',
        'django.middleware.locale.LocaleMiddleware',
        'django.middleware.common.CommonMiddleware',
    ],
    ROOT_URLCONF='i18n.i18n_unprefixed_urls',
    LANGUAGE_CODE='en',
)
class UnprefixedI18nSetLang(TestCase):
    def test_en2fr(self):
        self.client.post('/i18n/setlang/', data={'language': 'en'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'fr', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/fr/admin/', 302), ('/fr/admin/login/?next=/fr/admin/', 302)]
        )

    def test_fr2en(self):
        self.client.post('/i18n/setlang/', data={'language': 'fr'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'en', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/admin/', 302), ('/admin/login/?next=/admin/', 302)]
            )

    def test_fr2de(self):
        self.client.post('/i18n/setlang/', data={'language': 'fr'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'de', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/de/admin/', 302), ('/de/admin/login/?next=/de/admin/', 302)]
            )

comment:7 by George Tantiras, 7 years ago

Thank you, indeed. The docs https://docs.djangoproject.com/en/1.11/topics/i18n/translation/#the-set-language-redirect-view read:

After setting the language choice, Django looks for a next parameter in the POST or GET data. If that is found and Django considers it to be a safe URL (i.e. it doesn’t point to a different host and uses a safe scheme), a redirect to that URL will be performed. Otherwise, Django may fall back to redirecting the user to the URL from the Referer header or, if it is not set, to /, depending on the nature of the request:

Also the template that follows this quote - and I am using in the code where the problem appeared- has the following line:

input name="next" type="hidden" value="{{ redirect_to }}" />

I understand that in a custom view this is easy. However, it is not clear how can the variabe redirect_to can be set if this code is used in the admin interface, for example in the base.html admin template.

Last edited 7 years ago by George Tantiras (previous) (diff)

comment:8 by Tomer Chachamu, 7 years ago

Hi George, try adding the above context processor in your settings.TEMPLATES['OPTIONS']['context_processors'], see https://docs.djangoproject.com/en/1.11/topics/templates/#django.template.backends.django.DjangoTemplates

comment:9 by George Tantiras, 7 years ago

Bingo!

I managed to add it, and it works as expected.

Thank you!

comment:12 by George Tantiras, 5 years ago

Has patch: set

comment:13 by Mariusz Felisiak, 5 years ago

Component: InternationalizationDocumentation
Needs documentation: unset
Owner: set to George Tantiras
Status: newassigned
Version: 1.11master

comment:14 by Carlton Gibson, 5 years ago

Cc: Claude Paroz added
Component: DocumentationInternationalization
Has patch: unset

This looks like a bug to me, rather than a documentation issue.

You shouldn't have to set the next parameter, manually calling translate_url() to get the value, since the `set_language()` view does that itself:

                next_trans = translate_url(next, lang_code)
                if next_trans != next:
                    response = HttpResponseRedirect(next_trans)

A breakpoint inserted here, in the failing test case George provided (Original PR), shows the translate_url() call failing.

Furthermore, as per the report, the translate_url() call works when prefix_default_language=True.

The issue lies in the relation between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False.

If you disable LocaleMiddleware and set the the request language using translation.override() (to, e.g. 'el' here) the test passes.

The issue is in `LocaleMiddle.process_request()`:

    def process_request(self, request):
        urlconf = getattr(request, 'urlconf', settings.ROOT_URLCONF)
        i18n_patterns_used, prefixed_default_language = is_language_prefix_patterns_used(urlconf)
        language = translation.get_language_from_request(request, check_path=i18n_patterns_used)
        language_from_path = translation.get_language_from_path(request.path_info)
        if not language_from_path and i18n_patterns_used and not prefixed_default_language:
            language = settings.LANGUAGE_CODE
        translation.activate(language)
        request.LANGUAGE_CODE = translation.get_language()

Here, because the set_language() view is routed outside of i18n_patterns(), not language_from_path and i18n_patterns_used and not prefixed_default_language is True and so the language is always set to settings.LANGUAGE_CODE. This means that translate_url() never gets a match trying to resolve the url to translate (because the wrong language is activated).

IF you route the set_language() view inside i18n_patterns() then the test passes but you can't do that because it's explicitly warned against in the docs:

Warning
Make sure that you don’t include the above URL within i18n_patterns() - it needs to be language-independent itself to work correctly.

Two thoughts:

  • I'm not sure right now why that warning is required. (Could it be changed?)
  • translate_url() only functions on URLs coming from the currently active language. Would it be feasible to have set_language() redetermine the request language before calling translate_url()?
  • Is LocaleMiddleware's not language_from_path and i18n_patterns_used and not prefixed_default_language` correct? (I mean it seems it but...)

Option 2 seems the most likely. None of them look that nice.

Claude: I'd be grateful if you could check my reasoning here. 😬 Thanks.

comment:15 by Carlton Gibson, 5 years ago

Summary: Unclear documentation for 'next' parameter of set_language viewIncompatibility between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False.

comment:16 by Carlton Gibson, 5 years ago

#30272 is related here.

comment:17 by George Tantiras, 5 years ago

Owner: George Tantiras removed
Status: assignednew

A decision is pending about the suited course of action for this case.

comment:18 by Mariusz Felisiak, 12 months ago

#35009 was a duplicate.

comment:19 by Mariusz Felisiak, 12 months ago

#35034 was a duplicate.

comment:20 by Eric Soroos, 12 months ago

#35034 was marked as a duplicate -- however it's a little different, with no relationship to the prefix_default_language:
(note, this is in 4.2 as of the time of posting, but bug history suggests 1.x through 5.0)

When posting to /i18n/setlang/:

  • with a referrer in a language that's not the current django cooke language like /km/admin/foo/bar,
  • with a current language cookie of django_language=en
  • and a post content of next=&language=en

django.urls.translate_urls attempts to resolve the url using the current request language (in this case, en) and fails, because the url is in a different language (km).

i18n.set_language then returns the original referrer in the location header of the redirect, and the language doesn't apparently change, confusing the user.

This is approximately option 2 from https://code.djangoproject.com/ticket/28567#comment:14, but with the resolution entirely in translate_urls instead of set_language. The patch for translate_urls to handle alternate locales is approximately:

from django.utils.translation import override, check_for_language, get_language_from_path

...

def translate_url(url, lang_code):
    """                                                                                                                                                                            
    Given a URL (absolute or relative), try to get its translated version in                                                                                                       
    the `lang_code` language (either by i18n_patterns or by translated regex).                                                                                                     
    Return the original URL if no translated version is found.                                                                                                                     
    """
    parsed = urlsplit(url)
    try:
        # URL may be encoded.                                                                                                                                                      
        try:
            match = resolve(unquote(parsed.path))
        except Resolver404:
            url_lang_code = get_language_from_path(unquote(parsed.path))
            if url_lang_code and check_for_language(url_lang_code):
                with override(url_lang_code):
                    match = resolve(unquote(parsed.path))
            else:
                raise
    except Resolver404:
        pass
    else:
...

https://github.com/django/django/compare/main...EricSoroos:django:35034-translate-url

Last edited 12 months ago by Eric Soroos (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top