Opened 3 years ago

Closed 2 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 Changed 3 years ago by John D'Ambrosio

Owner: changed from nobody to John D'Ambrosio
Status: newassigned

comment:2 Changed 3 years ago by John D'Ambrosio

Has patch: set

comment:3 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:4 Changed 2 years ago by John D'Ambrosio

Version: 1.111.9

Adjusting versions affected--this has been around since recursive template loading was introduced in 1.9.

comment:5 Changed 2 years ago by John D'Ambrosio

Updated per feedback from Tim Graham.

comment:6 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In be68c0b:

Fixed #28071 -- Fixed {% extends %} origin history.

Note: See TracTickets for help on using tickets.
Back to Top