get_nodes_by_type refactor for easier extensibility (fixes an underlying bug for IfEqualNode and IfChangedNode)

[adam@…]

Block inheritance works fine within {% if %} tags, but not within {% ifequal %} or {% ifnotequal %} tags, as illustrated by the code below.

alpha.html
(the variable "t" is defined in the view as "true")

{% if t                 %} {% block stuff %}ALPHA{% endblock %} {% endif         %}
{% ifequal t "true"     %} {% block stuff %}ALPHA{% endblock %} {% endifequal    %}
{% ifnotequal t "false" %} {% block stuff %}ALPHA{% endblock %} {% endifnotequal %}

beta.html

{% extends "alpha.html" %}
{% block stuff %}BETA{% endblock %}

results

BETA
ALPHA
ALPHA

[Chris Beaven]

This patch fixes those (and {% for %}), factoring out some of the work back to NodeList, complete with tests.

[James Bennett]

Why not just put the conditional test inside the block? The existence of the block doesn't change based on the outcome of the conditional, only its contents, so it doesn't make sense to write it this way (and conditional definition of blocks is something Django has never really tried to support; AFAIK, if it works at all it works by accident).

[Chris Beaven]

This has nothing to do with conditionals or the "existence of a block", the issue is that a tag should be visible anywhere in a template by using the NodeList.get_nodes_by_type() method. For example:

{% for i in list %}
{% block forblock %}something{% endblock %}
{% endfor %}

The behavior of the ForNode and IfEqualNode are obviously broken if you take a closer look at how the IfNode.get_nodes_by_type() works (obviously not "accidental" code).

[James Bennett]

Your example doesn't make this any more clear to me; you can't recur a block of the same name multiple times, as that loop attempts to do. Similarly, you can't say "this block exists only if this condition is true".

You can say "inside this block, loop over this iterable and output that each time". And you can say, "inside this block, output this if that is true".

So I'm not sure why a block tag inside a conditional or a loop should be expected to work.

[Chris Beaven]

Replying to ubernostrum:

Your example doesn't make this any more clear to me; you can't recur a block of the same name multiple times, as that loop attempts to do. Similarly, you can't say "this block exists only if this condition is true".

A {% block %} tag just provides an extensible area for child templates. These {% block %} replacements happen before any actual rendering, be it conditional or iterable.

As an aside, what if you were looking for another type of tag - this patch would also fix those situations.

[Hystrix]

I've just been biten by this bug and I have to say it took me more than a day to find out why the child template didn't show up on the parent. If it won't be resolved at least it should be noted on the docs that you can't expect block tags to work inside ifequal.

[Hystrix]

Here's a use case for this, the view would pass a list of objects and some way to identify the active one. The parent template can render the list and set a block so the childs can modify only the active object with ease.

parent.html

<ol>
{% for o in objects %}<li>
  {% ifequal o active %}{% block active %}Object {{o}}{% endblock %}
  {% else %}{% include 'object.html' %}{% endifequal %}
  </li>{% endfor %}
</ol>

child1.html

{% extends 'parent.html' %}
{% block active %}Action 1 for {{o}}{% endblock %}

child2.html

{% extends 'parent.html' %}
{% block active %}Action 2 for {{o}}{% endblock %}

That way I don't have to write the for structure for every child

resutlt

Using child1.html with objects = ['a', 'b', 'c'], active = 'b'

<ol>
  <li>Object a</li>
  <li>Action 1 for b</li>
  <li>Object c</li>
</ol>

Using child2.html with objects = ['a', 'b', 'c'], active = 'a'

<ol>
  <li>Action 2 for a</li>
  <li>Object b</li>
  <li>Object c</li>
</ol>

[Chris Beaven]

PS: I just updated the patch to latest trunk.

And to anyone reviewing this ticket - note that the ticket description is a red herring. This is a valid bug even without a repeated {% block %} tag

[mMetry]

Hi guys. Any progress on this? I'm developing using Django v0.96 on the Google App Engine and this is coming up frequently. Any help would be much appreciated.

[xdcdx]

Did this finally get fixed? Somehow it is not yet working with Django 1.1.1

[Alexandre Garnier]

To link with ​http://code.djangoproject.com/ticket/10975

[Chris Beaven]

New patch against trunk, tests show the problem more clearly.

[Russell Keith-Magee]

Summarizing a discussion with SmileyChris on IRC:

The original problem that is alluded to (inconsistency between if and ifequal) no longer exists. The tests from the first patch (6510.diff) pass without any code modifications.

The current proposal in this ticket is to refactor and clean up the way child nodes are handled in template nodes, especially with the get_nodes_by_type() call. Calls to get_nodes_by_type() currently fail on ifequal and ifchanged nodes due to some inconsistencies in implementation. This doesn't have any obvious external manifestations, but in the interests of everything working consistently and avoiding nasty surprises later on, SmileyChris' patch does some cleanup.

[Chris Beaven]

Now with a test showing an external manifestation (probably what #10975 was talking about).

[Russell Keith-Magee]

(In [12655]) [1.1.X] Fixed #6510 -- Refactored the way child nodes are found in template nodes to avoid potential inconsistencies. Thanks to SmileyChris for the patch.

Backport of r12654 from trunk.

[Jacob]

Milestone 1.2 deleted