Opened 8 years ago
Closed 7 years ago
#28071 closed Bug (fixed)
ExtendsNode history initialized with context origin rather than own origin
Reported by: | John D'Ambrosio | Owned by: | John D'Ambrosio |
---|---|---|---|
Component: | Template system | Version: | 1.9 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
While the "extends" tag and blocks are typically used to extend a base template, one can also leverage it to extend an included template with blocks, as long as the rules of block inheritance are respected. A use case for this might be a base template that includes a menu stub:
{% include 'menu.html' %}
and then the bottom layer menu.html is something like
{% block menu-items %}
<li>Home</li>
{% endblock %}
and then an installed app higher in the load order makes its own menu.html as follows:
{% extends 'menu.html' %}
{% block menu-items %}
{{ block.super }}
<li>My App</li>
{% endblock %}
Current test coverage has some similar cases in that they a) make an included template with blocks and b) override those blocks, but there are currently no tests that validate that {{ block.super }} works as expected in that context.
Issue:
The current ExtendsNode behavior initializes the history from the context's origin instead of the node's origin. This results in it making two renders of the same origin, which in turn causes anything added above/below {{block.super}} to render twice.
In the example above, I would end up with:
<li>Home</li>
<li>My App</li>
<li>My App</li>
Proposed Solution:
By changing the initial history entry to be the node's origin, we ensure the behavior works correctly for both a base template and an included template.
In the solution provided, a simple test is added to validate the behavior. Before the change the test would fail because it would render the new content in the block twice (first time the origin in the history is for the context's base template, not the calling tag's template's origin), which results in it adding the node's origin the first pass, and then rendering it a second time before skipping it. This results in the string: "12ABB"
After the change, we get the expected result of: "12AB"
Change History (6)
comment:1 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 8 years ago
Has patch: | set |
---|
comment:3 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 8 years ago
Version: | 1.11 → 1.9 |
---|
Adjusting versions affected--this has been around since recursive template loading was introduced in 1.9.
https://github.com/django/django/pull/8234