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 , 7 years ago
Component: | Uncategorized → Internationalization |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 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')
comment:4 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 7 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 , 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 , 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.
comment:8 by , 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:13 by , 5 years ago
Component: | Internationalization → Documentation |
---|---|
Needs documentation: | unset |
Owner: | set to |
Status: | new → assigned |
Version: | 1.11 → master |
comment:14 by , 5 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'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 , 5 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 , 5 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
A decision is pending about the suited course of action for this case.
comment:20 by , 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
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.