Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12650 closed (fixed)

Some contrib apps depend on the i18n context processor to output valid XHTML

Reported by: robhudson Owned by: ramiro
Component: Contrib apps Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Specifically, the contrib.admin and contrib.databrowse expect the context of LANGUAGE_CODE to exist when outputting the <html> tag. Without these, the values for the lang attributes end up empty which is invalid XHTML. If someone disables i18n, would it be safe to assume en-us and convert those to: lang="{{ LANGUAGE_CODE|default:"en-us" }}"?

Attachments (1)

12650-r12947.diff (4.0 KB) - added by ramiro 5 years ago.
Patch fixing issue as devised by robhudson plus tests.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

What's the use case for the LANGUAGE_CODE setting not being there? It's in global_settings, and it's in the template for a new project - how is this problem manifesting for you?

comment:2 follow-up: Changed 5 years ago by robhudson

I made the assumption that if I'm not using i18n, I could also remove the i18n context processor, which is what provides the default settings to the template and why they are now blank in the admin and databrowse.

comment:3 in reply to: ↑ 2 Changed 5 years ago by ramiro

  • Triage Stage changed from Design decision needed to Accepted

Replying to robhudson:

I made the assumption that if I'm not using i18n, I could also remove the i18n context processor, which is what provides the default settings to the template and why they are now blank in the admin and databrowse.

And right you were, because you are doing something we've document as possible and even suggest to do so: http://docs.djangoproject.com/en/dev/topics/i18n/deployment/#if-you-don-t-need-internationalization (second paragraph).

comment:4 Changed 5 years ago by ramiro

  • Owner changed from nobody to ramiro
  • Status changed from new to assigned

comment:5 Changed 5 years ago by ramiro

  • Resolution set to worksforme
  • Status changed from assigned to closed

I started to work on this by first creating a test case that set the USE_I18N seting to False, removed 'django.core.context_processors.i18n' from the TEMPLATE_CONTEXT_PROCESSORS setting and then examined the content of the HTML tag of an admin page.

But it showed thing were working (the lang attributes had "en-us" values) even before applying any fix.

Looking at the Django code I see that the i18n context processor sets the LANGUAGE_CODE context var value from the result of a call to django.utils.translation.get_language() and django/utils/translation/trans_null.py (the one used when USE_i18N is False) has get_language = lambda: settings.LANGUAGE_CODE. So LANGUAGE_CODE never can have and empty value.

Tested this against trunk and 1.1.x as of now.

The only other possibility I seas a a reason for getting an empty LANGUAGE_CODE context var value is if you are explicitly deleting the LANGUAGE_CODE setting or setting it to an empty value which isn't the right way to deactivate i18n machinery.

Because of this I'm closing this ticket as worksforme. Please reopen If I've missed anything.

comment:6 Changed 5 years ago by robhudson

I just looked again at 2 of my projects using debug toolbar. If I add the i18n context processor, I see the LANGUAGE_CODE context variable in the templates panel and the HTML looks good. When I disable it, I no longer see the LANGUAGE_CODE available in the context. Checking the settings panel, I do, in fact see that LANGUAGE_CODE='en-us'... it's just not getting into the template context. And if the i18n context processor is disabled, I don't see how it would get there unless manually added in the view.

Unless someone else can see this, I'm not going to re-open it.

Changed 5 years ago by ramiro

Patch fixing issue as devised by robhudson plus tests.

comment:7 Changed 5 years ago by ramiro

  • Component changed from Uncategorized to Contrib apps
  • Has patch set
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Please ignore all my babbling about where the i18n context processor gets the value of LANGUAGE_NAME from, the point is such context processor not being present at all.

As for why the test case I had created was showing the LANGUAGE_CODE=en value was present in the context of the admin, the reason was:

  • The test machinery hard-codes LANGUAGE_NAME to 'en'.
  • The i18n context processor is in the default TEMPLATE_CONTEXT_PROCESSORS setting set by global_settings.py
  • The internal list of context processors is generated once from TEMPLATE_CONTEXT_PROCESSORS and then cached.

This explains why the XHTML header contained 'lang="en" xml:lang="en"' showing no failure (OTOH Rob's and my own live testing with the admin app showed invalid XHTML headers were being generated). Also, the test passed/failed depending if another test case with was run before or not.

Even when I was munging with settings.TEMPLATE_CONTEXT_PROCESSORS to remove the i18n context processor, that change had no effect in the processing of the request made by the test client. I solved this in a somewhat hacky way: I reset the internal cache of context processors used by RequestContext after I modify that setting.

Thanks Rob for insisting on this.

comment:8 Changed 5 years ago by ramiro

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [14104]) Fixed #12650 -- Don't generate invalid XHTML in the admin, databrowse apps when
the i18n context processor is active. Thanks to Rob Hudson for the report and
fix suggestion.

comment:9 Changed 5 years ago by ramiro

(In [14105]) [1.2.X] Fixed #12650 -- Don't generate invalid XHTML in the admin, databrowse apps when
the i18n context processor is active. Thanks to Rob Hudson for the report and
fix suggestion.

Backport of [14104] from trunk

Note: See TracTickets for help on using tickets.
Back to Top