#2384 closed defect (fixed)
Changing TEMPLATE_STRING_IF_INVALID breaks Admin Interface display of model list
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | normal | Keywords: | |
Cc: | cmawebsite@… | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Model functions (as opposed to fields) used in "list_display" for the Admin interface lack the attribute "class_attrib". If TEMPLATE_STRING_IF_INVALID is changed from the default "" the new string is inserted at the end of the column's <th> tag, causing the model's list table to break on rendering.
Affects Admin interface template:
contrib/admin/templates/admin/change_list_results.html
Change History (4)
comment:1 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 by , 12 years ago
Easy pickings: | unset |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
UI/UX: | unset |
Well, that doesn't actually fix the problem, it just documents it.
In https://github.com/django/django/blob/master/django/contrib/admin/templatetags/admin_list.py#L109, result_headers
yields a text block without a class_attrib
value. But the template expects it to be present. We could just have it send an empty one.
comment:3 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
We basically gave up attempting to fix this bug. Because no-one ever uses non-default values of TEMPLATE_STRING_IF_INVALID in production, no-one ever catches bugs caused by non-default values, so the bug always comes back, in the same place or a different place. Attempting to fix it will just waste more time.
comment:4 by , 12 years ago
Cc: | added |
---|
I would be interested in revisiting the policy on TEMPLATE_STRING_IF_INVALID
. This is an optional setting that can make the template language much more strict. It has shown me lots of template bugs on my websites that I didn't realize existed, and I trust it enough to even use it in production, despite the recommendation. I would recommend to people to use it more, if only the django admin didn't rely on silent invalid variable errors.
We could use a non-default value in our admin tests to be sure these types of errors do not crop up again.
Is it worth fixing the admin? Yes. As far as I can tell, the admin is actually very close to being 100% free relying on invalid variables. Our original policy on TEMPLATE_STRING_IF_INVALID
was put in place back in the old-admin days, but things are better now. As far as I've seen on live websites it's really just header.class_attrib
. I have USE_I18N = False
set, so the LANGUAGE_CODE
variable is also invalid. On a live production website, those are the only two invalid variables I have run into.
It's easy to fix these errors, just wrap them in {% if header.class_attrib %}
.
Here's what I use as my "string" in my settings. Note the workaround for this ticket:
class InvalidVar(object): def __mod__(self, s): assert s.var in ('LANGUAGE_CODE', 'header.class_attrib'), s.var return '' def __contains__(self, x): return x == '%s' TEMPLATE_STRING_IF_INVALID = InvalidVar()
(In [3901]) Fixes #2384,#2566 -- Clarify the role that TEMPLATE_STRING_IF_INVALID plays in the template system, and the problems that can occur if it is used on a production site.