Template if/ifequal/ifnotequal evaluate despite condition

[Joshua 'jag' Ginsberg <jag@…>]

The problem:

Django evaluates all branches of an if/else/endif regardless of the condition. Most of the time, given the forgiving nature of templates, this is fine. However sometimes this is really bad.

root@agilus:/srv/django/satchmo# ./manage.py shell
Python 2.5.1 (r251:54863, Mar  7 2008, 03:19:34) 
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from satchmo.shop.templatetags import satchmo_util
>>> satchmo_util.app_enabled("recentlist")
''
>>> from django.template import Template, RequestContext
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}Enabled{% endif %}""")
>>> from django.template import Context
>>> ctx = Context({})
>>> t.render(ctx)
u''
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}{% load satchmo_recentlist %}{% endif %}""")
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 166, in __init__
    self.nodelist = compile_string(template_string, origin)
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 187, in compile_string
    return parser.parse()
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 271, in parse
    compiled_result = compile_func(self, token)
  File "/usr/local/lib/python2.5/site-packages/django/template/defaulttags.py", line 764, in do_if
    nodelist_true = parser.parse(('else', 'endif'))
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 271, in parse
    compiled_result = compile_func(self, token)
  File "/usr/local/lib/python2.5/site-packages/django/template/defaulttags.py", line 857, in load
    (taglib, e))
TemplateSyntaxError: 'satchmo_recentlist' is not a valid tag library: Could not load template library from django.templatetags.satchmo_recentlist, No module named satchmo_recentlist

Django shouldn't have even evaluated the {% load satchmo_recentlist %} because the condition clearly fails. The same bug exists for ifequals and ifnotequals.

With the attached patch applied, we see the following behavior, which I believe is correct:

root@agilus:/srv/django/satchmo# ./manage.py shell
Python 2.5.1 (r251:54863, Mar  7 2008, 03:19:34) 
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from satchmo.shop.templatetags import satchmo_util
>>> satchmo_util.app_enabled("recentlist")
''
>>> from django.template import Template, Context
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}Enabled{% endif %}""")
>>> ctx = Context({})
>>> t.render(ctx)
u''
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}{% load satchmo_recentlist %}{% endif %}""")
>>> t.render(ctx)
u''
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}{% load satchmo_recentlist %}{% else %}{% load nonexistant_tags %}{% endif %}""")
>>> t.render(ctx)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 176, in render
    return self.nodelist.render(context)
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 751, in render
    bits.append(self.render_node(node, context))
  File "/usr/local/lib/python2.5/site-packages/django/template/debug.py", line 71, in render_node
    result = node.render(context)
  File "/usr/local/lib/python2.5/site-packages/django/template/defaulttags.py", line 251, in render
    raise self.nodelist_false
TemplateSyntaxError: 'nonexistant_tags' is not a valid tag library: Could not load template library from django.templatetags.nonexistant_tags, No module named nonexistant_tags
>>> t = Template("""{% load satchmo_util %}{% if not "recentlist"|app_enabled %}{% load satchmo_recentlist %}{% endif %}""")
>>> t.render(ctx)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 176, in render
    return self.nodelist.render(context)
  File "/usr/local/lib/python2.5/site-packages/django/template/__init__.py", line 751, in render
    bits.append(self.render_node(node, context))
  File "/usr/local/lib/python2.5/site-packages/django/template/debug.py", line 71, in render_node
    result = node.render(context)
  File "/usr/local/lib/python2.5/site-packages/django/template/defaulttags.py", line 246, in render
    raise self.nodelist_true
TemplateSyntaxError: 'satchmo_recentlist' is not a valid tag library: Could not load template library from django.templatetags.satchmo_recentlist, No module named satchmo_recentlist
>>> t = Template("""{% load satchmo_util %}{% if "recentlist"|app_enabled %}{% load satchmo_recentlist %}{% else %}Disabled{% endif %}""")
>>> t.render(ctx)
u'Disabled'
>>> t = Template("""{% load satchmo_util %}{% ifequal 3 4 %}{% load satchmo_recentlist %}{% else %}Disabled{% endifequal %}""")
>>> t.render(ctx)

Thanks!

-jag

[John Shaffer]

There was a discussion on the django-developers list: ​http://groups.google.com/group/django-developers/browse_thread/thread/bf84b665d9c8728e/0354321e65aa2ace

[Jeremy Dunck]

IMHO, this isn't as bad as it sounds; summarizing the thread, what's really going on here is that the load tag has a side-effect in the *parse* phase of template usage; this affects the example because app_enabled returns true because of that side effect.

Introducing a {% loadif %} tag (as suggested in the thread) is a bit late for 1.0, I think, and not backwards incompatible.

[Malcolm Tredinnick]

This would require a fairly intrusive change to the template parsing code from a first look at it. Not appropriate for right now.

Adding new tags after 1.0 is tricky, too, since it potentially introduces a name clash with a third-party tag, breaking existing code (and hence not being backwards-compatible). That part needs some thought. Bumping the main issue to post-1.0 in any case. The current workaround is "don't do that" and/or writing your own loading tag.

[(none)]

Milestone post-1.0 deleted

[Chris Beaven]

I'm marking as wontfix because the entire template is parsed no matter what conditionals there are, and this shouldn't change.
As already stated, the problem the user is having here is that {% load %} behaviour happens at parse time.

From the discussion in the list:

However, I don't believe loading a non-existent library should be a syntax error any more than accessing a non-existent key in a dictionary.

That's a fair suggestion. Something like {% load mylibrary fail-silently %} perhaps? (with my suggestion being fail-silently since isn't a valid module name, avoiding any chance of a clash)
But that's a separate issue and if someone feels strongly about it, they can open a new ticket.

[Adam McMaster]

This should be fixed, because it really can cause problems. For example, I have the following in a template:

{% if category %}
{% block backtolink %}<a href="{% url community.views.category slug=category.slug page=frompage%}">&larr; Back to &quot;{{ category }}&quot;</a>{% endblock %}
{% endif %}

This raises NoReverseMatch when "category" is not set, so I'm now left having to add extra cruft to my views to get around it.

[Łukasz Rekucki]

From my understanding, blocks are replaced before conditions are evaluated by design. You can move the if tag to the block like this:

{% block backtolink %}
{% if category %} {# replace block with new content if there is a category #}
    <a href="{% url community.views.category slug=category.slug page=frompage%}">&larr; Back to &quot;{{ category }}&quot;</a>{% endblock %}
{% else %}{{ block.super }}{% endif %} {# leave the block alone #}
{% endblock %}

[Chris Beaven]

Yes, it's by design and very fundamental to how the template system works.