TranslationMiddleware doesn't remember language choice

[mvdwaeter]

Issue summary

Not sure if currently bug or feature, but I'd expect that the TranslationMiddleware reads and sets a cookie with LANGUAGE_COOKIE_NAME, so that the browser remembers to redirect to /nl/ or /en/ when going to /.

How to reproduce?

Basic setup with wagtailtrans, using TranslationMiddleware with two languages, let's say nl and en. The Homepage on / redirects to /nl/ or /en/ based on browser headers. I have an English browser, so I'm redirected to /en/. I now choose to go to /nl/. I now go to / and I expect to go to /nl/ but I'm redirected to /en/.

Technical details

Django==2.0.5 wagtail==2.0.1 wagtailtrans==2.0.1

[mvdwaeter]

This is my own implementation of the middleware that does remember the chosen language by setting a cookie.

from django.conf import settings
from django.utils import translation
from django.utils.deprecation import MiddlewareMixin

from .models import Language
from .sites import get_languages_for_site

class TranslationMiddleware(MiddlewareMixin):
    def process_request(self, request):
        active_language = None
        language_from_path = translation.get_language_from_path(request.path)
        cookie_language = translation.get_language_from_request(request)
        requested_languages = request.META.get('HTTP_ACCEPT_LANGUAGE')
        if language_from_path:
            active_language = language_from_path
        elif cookie_language:
            active_language = cookie_language
        elif requested_languages:
            requested_languages = requested_languages.split(',')
            codes = tuple(language.code for language in get_languages_for_site(request.site) if language)

            for language in requested_languages:
                language = language.split(';')[0]
                active_language = language if language in codes else None

                if active_language is None and language.startswith(codes):
                    active_language = [c for c in codes if language.startswith(c)][0]
                if active_language is not None:
                    break

        if active_language is None:
            default_language = Language.objects.default_for_site(site=request.site)

            if default_language:
                active_language = default_language.code
            else:
                active_language = settings.LANGUAGE_CODE

        translation.activate(active_language)
        request.LANGUAGE_CODE = active_language

    def process_response(self, request, response):
        if 'Content-Language' not in response:
            response['Content-Language'] = request.LANGUAGE_CODE
        return response

[Henk-JanVanHasselaar]

@mvdwaeter Thank you for logging this issue.

Right now this is not part of WagtailTrans's functionality. The idea is that once you've been redirected to the /en or /nl you have no need to go back to the / page.

For every request the middleware then checks if you have a language code in your path, if that's not the case it checks for the browser language.

For what cases would you need the language cookie if you use i18n urls and your page trees starts with the language codes?

[mvdwaeter]

Because users will always go to / when opening a new tab or window. Their browser setting might not be the preferred language for a website.

[Henk-JanVanHasselaar]

That makes sense, also when the user comes back after a previous session you'd want to restore the previously selected language. When do you propose setting the language cookie? After switching languages using the language selector?

also, get_language_from_request() will check the HTTP_ACCEPT_LANGUAGE and can also check the path if you pass check_path=True. Also this method will eventually fall back to language defined in settings.LANGUAGE_CODE so we'd have to make a custom implementation which will also check for Wagtail Trans's default language.

Would you like to cook up a PR for this?

[mvdwaeter]

https://github.com/LUKKIEN/wagtailtrans/pull/125

[mikedingjan]

Thanks for your contribution @mvdwaeter Since your PR #125 is merged, I'm closing this issue.