[Django] #30272: get_language_from_path not respecting i18n_patterns prefix_default_language=False
Django
prefix_default_language=False
------------------------------------------------+------------------------
Reporter: Stefan Wehrmeyer | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Starting in Django 1.10 `django.conf.urls.i18n.i18n_patterns` accepts an
option `prefix_default_language` that can be set to `False`. This adds no
language prefix for the the default language when reversing URLs.
`django.utils.translation.get_language_from_path` is supposed to detect
the language from the URL path. At the moment it returns `None` when no
language URL prefix is found. I would argue that if the path is resolved
by `i18n_patterns` with `prefix_default_language=False`
`get_language_from_path` should return the default language
(`settings.LANGUAGE_CODE`)
I believe the intention of using `i18n_patterns` and setting
`prefix_default_language=False` is two-fold:
- construct URLs in the default language without a language prefix (this
works now)
- detect non-prefixed URLs as using the default language (currently *not*
working, falls back to session, cookie, HTTP headers)
Let me know if you consider this a bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Component: | Version: 2.1
Internationalization |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
Hi Stefan.
I think this is intended behaviour: doing clever things, like falling back
to the default `LANGUAGE_CODE` is out of scope for this function. It's job
it just (literally) to get the language code from the path. If it's not
there then `None` is the desired response.
The more sophisticated `get_language_from_request()` will do the ''right
thing'' here and, ultimately, return `settings.LANGUAGE_CODE` if nothing
else preempts it.
(Was there an incorrect behaviour you observed here, beyond this?)
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:1>
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Component: | Version: 2.1
Internationalization |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Stefan Wehrmeyer):
Hey Carlton, thanks for reviewing.
The problem with `get_language_from_request` is that the browser `Accept-
Language` header is very likely set and will determine the language.
However, I want the language to be determined by the path – even when
`prefix_default_language=False`. This works with
`prefix_default_language=True`.
`LocaleMiddleware` actually does this, but does it around
`get_language_from_request`:
So maybe an idea could be to move these lines into
`get_language_from_request` in order to make this logic available to other
Django apps that rely on this API (in my case django cms). Not sure if
this would be a backwards-incompatible change though.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:2>
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Component: | Version: 2.1
Internationalization |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Why wouldn't `get_language_from_path() or settings.LANGUAGE_CODE` do you?
(Just trying to understand the use-case.)
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:3>
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Component: | Version: 2.1
Internationalization |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Stefan Wehrmeyer):
Sorry, some more details about the use case.
I'm running a Django CMS instance with their URLs included via
`i18n_patterns` and `prefix_default_language=False`. I want the URL path
to determine what language to serve (and `LocaleMiddleware` set this
language correctly!).
The problem is that Django CMS relies on `get_language_from_request`
([https://github.com/divio/django-
cms/blob/eeb1e4712b3866e243daf800c142e2199e4be9df/cms/views.py#L85 e.g.
here]) to detect which language to serve without the additional logic that
is included in `LocaleMiddleware` to special case
`prefix_default_language=False`.
Example URLs with default language "en":
- `/de/ueber/` – `LocaleMiddleware` sets "de", `get_language_from_request`
with `check_path=True` returns "de"
- `/about/` – `LocaleMiddleware` sets "en", `get_language_from_request`
with `check_path=True` returns whatever is in session, language cookie or
HTTP `Accept-Language` header, but should return "en" like
`LocaleMiddleware`
This leads in turn to the wrong language being served. This could
certainly be solved in Django CMS as well.
But currently there are cases when `LocaleMiddleware` will set a different
language on the request than is returned by `get_language_from_request` –
which makes this public API inconsistent IMHO.
Thanks for taking the time to look into this. I can try to provide a test
case if this makes it clearer.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:4>
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Type: Bug | Status: new
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: closed => new
* resolution: wontfix =>
Comment:
OK, I'll re-open to have another look. (If you do have that test case,
that would be super.)
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:5>
Django
prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Type: Bug | Status: new
Internationalization |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Stefan Wehrmeyer):
I added a test case here:
https://github.com/django/django/pull/11123
I created the new `UnprefixedDefaultLanguageUtilsTests` based on the
`UnprefixedDefaultLanguageTests` class (which is testing
`LocaleMiddleware` behaviour on i18n URLs without prefix). All methods
pass except `test_default_lang_without_prefix` ([https://djangoci.com/job
/pull-requests-bionic/database=postgis,label=bionic-
pr,python=python3.7/4322/testReport/ e.g. one test on DjangoCI]).
This could be resolved by making the language detection of
`LocaleMiddleware` and `get_language_from_request` behave the same way.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:6>
Django
i18n_patterns prefix_default_language=False
--------------------------------------+------------------------------------
Reporter: Stefan Wehrmeyer | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: 2.1
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Hi Stefan,
I'm going to provisionally accept this, but ask for a review from others.
Looking at 85a4844f8a8e628b90fa30ba7074f162a2d188ef, it's clear that we
should (and do) serve the default language, ignoring `Accept-Language`,
so, yes, plausibly, matching that in `get_language_from_request()`, when
using `check_path`, would be correct.
I've
[https://github.com/django/django/pull/11123/commits/eaaa07ca971c2d19bc166e0750d043a49ba575fd
added a very minimal port of the logic from `LocaleMiddleware` to
`get_language_from_request()` on your PR]. Suitably tidied this might
resolve the issue. (We'll see what CI says, but the change looks like it
doesn't break anything...)
I've retitled the issue: `get_language_from_path()` doesn't have access to
e.g. the `request`, so can't house the right logic here.
`get_language_from_request()` is our target.
Thanks for taking the time to explain the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:7>
Django
i18n_patterns prefix_default_language=False
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Internationalization | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Rodrigo):
The idea seems right to me
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:8>
Django
i18n_patterns prefix_default_language=False
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Internationalization | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* needs_better_patch: 1 => 0
Comment:
Landed here from surfing GitHub. Don't see comments here or on the PR for
the author to address, so resetting review flags for now. (Forgive me
Carlton if I missed something!)
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:9>
Django
i18n_patterns prefix_default_language=False
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Internationalization | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Claude Paroz (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:10>
Django
i18n_patterns prefix_default_language=False
--------------------------------------+------------------------------------
Component: Internationalization | Version: 2.1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
OK, with time to come back to this I'm going to say `wontfix` here. The
current behaviour is correct.
> But currently there are cases when LocaleMiddleware will set a different
language on the request than is returned by get_language_from_request –
which makes this public API inconsistent IMHO.
Right, sure, but `get_language_from_request()` operates over **just the
request** whereas `LocaleMiddleware` operates over the **request plus your
site configuration**. These aren't the same inputs, so it's not expected
that they have the same results.
> This could certainly be solved in Django CMS as well.
Yes. I think that's the correct response, and indeed, [https://github.com
/django-cms/django-cms/pull/6851/files#diff-
451cc2fd73e55d14cc72681ebdd8c201021f27c60ecdebb55760d2bb6c8794b6L89-R92
that's what they've done].
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:11>
Django
i18n_patterns prefix_default_language=False
Reporter: Stefan Wehrmeyer | Owner: nobody
Component: | Version: 2.1
Internationalization |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* cc: Claude Paroz (removed)
* stage: Accepted => Unreviewed
--
Ticket URL: <https://code.djangoproject.com/ticket/30272#comment:12>