Opened 8 years ago
Last modified 23 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 , 8 years ago
| Component: | Uncategorized → Internationalization |
|---|---|
| Type: | Uncategorized → Bug |
comment:2 by , 8 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')
comment:4 by , 8 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 8 years ago
| Needs documentation: | set |
|---|---|
| Owner: | removed |
| Status: | assigned → new |
| Summary: | Set Language Redirect View -> 404 if prefix_default_language=False → Unclear documentation for 'next' parameter of set_language view |
| Triage Stage: | Unreviewed → Accepted |
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 , 8 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 , 8 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.
comment:8 by , 8 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:13 by , 6 years ago
| Component: | Internationalization → Documentation |
|---|---|
| Needs documentation: | unset |
| Owner: | set to |
| Status: | new → assigned |
| Version: | 1.11 → master |
comment:14 by , 6 years ago
| Cc: | added |
|---|---|
| Component: | Documentation → Internationalization |
| 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 haveset_language()redetermine the request language before callingtranslate_url()?- Is
LocaleMiddleware'snot 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 , 6 years ago
| Summary: | Unclear documentation for 'next' parameter of set_language view → Incompatibility between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False. |
|---|
comment:17 by , 6 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
A decision is pending about the suited course of action for this case.
comment:20 by , 23 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
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.