Opened 9 years ago

Closed 3 years ago

Last modified 2 years ago

#2384 closed defect (fixed)

Changing TEMPLATE_STRING_IF_INVALID breaks Admin Interface display of model list

Reported by: robbie@… Owned by: adrian
Component: contrib.admin Version: master
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 Changed 9 years ago by russellm

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

(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.

comment:2 Changed 3 years ago by gcc

  • Easy pickings unset
  • Resolution fixed deleted
  • Status changed from closed to 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 Changed 3 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from reopened to 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 Changed 2 years ago by CollinAnderson

  • Cc cmawebsite@… 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()
Version 0, edited 2 years ago by CollinAnderson (next)
Note: See TracTickets for help on using tickets.
Back to Top