get_language_from_request(..., check_path=True) not respecting i18n_patterns prefix_default_language=False

[Stefan Wehrmeyer]

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.

[Carlton Gibson]

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.

[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:

​https://github.com/django/django/blob/cbf7e71558c94ce1c327b683768a18a25d82d197/django/middleware/locale.py#L19-L24

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.

[Carlton Gibson]

Why wouldn't get_language_from_path() or settings.LANGUAGE_CODE do you? (Just trying to understand the use-case.)

[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 (​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.

[Carlton Gibson]

OK, I'll re-open to have another look. (If you do have that test case, that would be super.)

[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 (​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.

[Carlton Gibson]

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 in this kind of case, so, yes, plausibly, matching that in get_language_from_request(), when using check_path, would be correct.

I've ​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.

[Rodrigo]

The idea seems right to me

[Jacob Walls]

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!)

[Carlton Gibson]

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, ​that's what they've done.