#14825 closed New feature (fixed)
LocaleMiddleware should store language preferences if possible
Reported by: | Vlastimil Zíma | Owned by: | Vlastimil Zíma |
---|---|---|---|
Component: | Internationalization | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | raymond.penners@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
After logout (or other action than flushes session) language preferences are forgotten. This leads to strange behaviour when page during which session was flushed (e.g. /adminsite/logout/) is translated but next page is in default language (or browser prefered language).
This is tricky especially when your system requires explicit logout to log in other user.
Solution is quite easy, LocaleMiddleware should store current language when it is not stored in session or language cookie.
Attachments (2)
Change History (18)
by , 14 years ago
Attachment: | patch-14825.diff added |
---|
follow-up: 2 comment:1 by , 14 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Design decision needed |
Marking this as Design Decision Needed, cause despite I understand your use case when the session is flushed one might argue that the chosen language is sensitive data that needs to be flushed.
Another solution to your problem would be to render the "logged out" template before flushing the session.
comment:2 by , 14 years ago
Replying to rasca:
Marking this as Design Decision Needed, cause despite I understand your use case when the session is flushed one might argue that the chosen language is sensitive data that needs to be flushed.
Another solution to your problem would be to render the "logged out" template before flushing the session.
So it does not have to be the same middleware.
I do not see the point in rendering "logged out" page, because if you refresh it (GET request) you get different result (with default language) and any other page where you go from there will also have default language.
Only reasonable thing, I can think of, is call set_language after logout so it sets language to default before rendering the "logged out" page.
comment:3 by , 14 years ago
Patch needs improvement: | set |
---|
Patch should be generated from the root of the tree, it isn't, and it seems to be in reversed patch. Also, would be nice if it has some context (e.g. changing its format to unified)
comment:5 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
Marking as 'new feature' as it changes current behavior; please also consider updating relevant documentation.
comment:6 by , 13 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
comment:8 by , 12 years ago
comment:9 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
This is a reasonable short-term workaround. #15902 will certainly take more time.
The patch is a good starting point, but it needs tests. Also, please use in
rather than has_key
, and consider using setdefault
where possible.
I view this as a bugfix more than a new feature, but a line in the release notes can't hurt.
comment:10 by , 12 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Owner: | changed from | to
Patch needs improvement: | unset |
Status: | new → assigned |
I updated the patch and created a pull request https://github.com/django/django/pull/957
I also realised that this only affects sessions. LocaleMiddleware
should not touch the language cookie in this case, because it is not changed by session flush.
comment:11 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:12 by , 12 years ago
Triage Stage: | Ready for checkin → Accepted |
---|---|
Version: | 1.2 → master |
Please don't mark your own patch as RFC, someone else should review your work.
comment:13 by , 11 years ago
This looks good to me. Unfortunately the patch no longer applies after the sprints we just had. Could you bring it up to date and mark it as RFC once that's done? (Since I reviewed an almost final version, as an exception, you can mark it as RFC by yourself.)
comment:14 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:15 by , 11 years ago
This change has made multi-language testing much more difficult for authenticated Django applications.
Prior to this commit, my QA team could follow this simple process to switch languages during testing:
- Switch language in browser settings
- Refresh page
- Test
Now, they must do this:
- Switch language in browser settings.
- Clear session cookie
- Re-authenticate
- Navigate to page under test
- Test
This behavior seems ok for the average user (whose language doesn't change minute to minute), but for a tester, this really makes things slow. Even test automation suffers (e.g., Selenium).
Could we at least put this behavior being a setting? Something like setting.STORE_LANGUAGE_IN_SESSION = True|False (default True)
comment:16 by , 11 years ago
There is a ticket about this regression, see #21473. The suggested solution is to not flush language from session on logout.
LocaleMiddleware patch