Opened 8 months ago

Last modified 8 months ago

#30272 new Bug

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

Reported by: Stefan Wehrmeyer Owned by: nobody
Component: Internationalization Version: 2.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

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.

Change History (7)

comment:1 Changed 8 months ago by Carlton Gibson

Resolution: wontfix
Status: newclosed

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.

comment:2 Changed 8 months ago 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:

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.

comment:3 Changed 8 months ago by Carlton Gibson

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

Last edited 8 months ago by Carlton Gibson (previous) (diff)

comment:4 Changed 8 months ago 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 (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.

comment:5 Changed 8 months ago by Carlton Gibson

Resolution: wontfix
Status: closednew

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

comment:6 Changed 8 months ago 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 (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.

comment:7 Changed 8 months ago by Carlton Gibson

Has patch: set
Patch needs improvement: set
Summary: get_language_from_path not respecting i18n_patterns prefix_default_language=Falseget_language_from_request(..., check_path=True) not respecting i18n_patterns prefix_default_language=False
Triage Stage: UnreviewedAccepted

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.

Last edited 8 months ago by Carlton Gibson (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top