Opened 22 months ago

Last modified 13 days ago

#28567 assigned Bug

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

Reported by: George Tantiras Owned by: George Tantiras
Component: Internationalization Version: master
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 (14)

comment:1 Changed 22 months ago by Tim Graham

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 Changed 22 months ago by George Tantiras

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 22 months ago by George Tantiras (previous) (diff)

comment:3 Changed 22 months ago by George Tantiras

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

comment:4 Changed 20 months ago by Tomer Chachamu

Owner: changed from nobody to Tomer Chachamu
Status: newassigned

comment:5 Changed 20 months ago by Tomer Chachamu

Needs documentation: set
Owner: Tomer Chachamu deleted
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 Changed 20 months ago by Tomer Chachamu

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 Changed 20 months ago by George Tantiras

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 20 months ago by George Tantiras (previous) (diff)

comment:8 Changed 20 months ago by Tomer Chachamu

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 Changed 20 months ago by George Tantiras

Bingo!

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

Thank you!

comment:12 Changed 2 weeks ago by George Tantiras

Has patch: set

comment:13 Changed 2 weeks ago by felixxm

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

comment:14 Changed 13 days ago by Carlton Gibson

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 Changed 13 days ago by Carlton Gibson

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 Changed 13 days ago by Carlton Gibson

#30272 is related here.

Note: See TracTickets for help on using tickets.
Back to Top