Opened 9 years ago

Closed 9 years ago

#5124 closed (fixed)

{% block %} inheritance not working after a few levels of {% extends %} (includes minimal demo app)

Reported by: vdupras@… Owned by: k0001
Component: Template system Version: master
Severity: Keywords: extends block load templatetags
Cc: favo@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

This is a very, very, *very* strange and obscur bug. When I encountered it, I thought it was a simple {% extends %} and {% block %} misbehavior, but when I tried to build a minimal app to reproduce it, I couldn't. In fact, it has to do with extends, block *and* load (for templatetags).

I can't explain the bug, but please, someone, look at the demo app. "template6" has a block containing "THIS IS WHAT SHOULD BE OUTPUT BUT ISN'T". When you run the app, this text is not output.

If you remove the "{% load mytags %}" line from template5.html, oops! the text will be there.

*OR*

If you add "{% block css %}{{ block.super }}{% endblock %}" in template5.html, oops! the text is back again.

Sorry for not having simplified the demo further, I've been going crazy to try to reproduce the bug so I simply copy/pasted my template hierarchy and replaced text with dummy text.

Attachments (3)

must_be_first.diff (1.6 KB) - added by k0001 9 years ago.
adds support for a new Node property "must_be_first"
contains_nontext.diff (665 bytes) - added by Wayne K. Werner 9 years ago.
contains_nontext_2.diff (675 bytes) - added by Wayne K. Werner 9 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 9 years ago by vdupras@…

Why can't I attach my file to the ticket?

comment:2 Changed 9 years ago by vdupras@…

Oh well, it's definetly a buggy day for Django, when I try to attach a file I get a pgsql error. Well, until I can attach my file, you can get it there:

http://s3.amazonaws.com:80/goombah/extends_bug.zip

comment:3 Changed 9 years ago by James Bennett

You cannot attach .zip or .tar.gz files here; the preferred format is a single .diff.

comment:4 Changed 9 years ago by anonymous

Owner: changed from nobody to anonymous
Status: newassigned

comment:5 Changed 9 years ago by Yuri Baburov

Owner: changed from anonymous to Yuri Baburov
Status: assignednew

comment:6 Changed 9 years ago by Yuri Baburov

Status: newassigned

Found the problem, it's related to the following code in source:django/trunk/django/template/loader_tags.py#L64:

class ExtendsNode(Node):
    ...
    def render(self, context):
        compiled_parent = self.get_parent(context)
        parent_is_child = isinstance(compiled_parent.nodelist[0], ExtendsNode)
        parent_blocks = dict([(n.name, n) for n in compiled_parent.nodelist.get_nodes_by_type(BlockNode)])

parent_is_child is set to False, when nodelist[0] node is {% load "..." %} block, and mystics occurred.
I think correct solution should search for existance of ExtendsNode in nodelist, instead of picking up nodelist[0].
Continuing investigations to be sure my proposed change won't break anything.

comment:7 Changed 9 years ago by Yuri Baburov

Triage Stage: UnreviewedDesign decision needed

Core devs, please decide what to do:

  • throw error that ExtendsNode should always be the first node (i think there was one)

or

  • search ExtendsNode through blocks

Patches for both decisions are very small.

comment:8 Changed 9 years ago by Yuri Baburov

Triage Stage: Design decision neededAccepted

Decision by Malcolm:
throw error that ExtendsNode? should always be the first node (i think there was one)
Problem:
I can't find good enough place to put it.

Changed 9 years ago by k0001

Attachment: must_be_first.diff added

adds support for a new Node property "must_be_first"

comment:9 Changed 9 years ago by k0001

This patch adds support for a new Node property "must_be_first", which when set in a Node subclass (like ExtendsNode) means that that node should be the first one on the NodeList, if it isnt' it raises a TemplateSyntaxError when trying to add that node into the nodelist.

Thanks SmileyChris for your help on this =)

comment:10 Changed 9 years ago by k0001

Resolution: fixed
Status: assignedclosed

comment:11 Changed 9 years ago by Chris Beaven

Resolution: fixed
Status: closedreopened
Triage Stage: AcceptedReady for checkin

Thanks for the patch, k0001, but it's not fixed until it is applied.

I will however promote it to ready for checkin, because it does everything that Malcolm wants.

comment:12 Changed 9 years ago by Yuri Baburov

Owner: changed from Yuri Baburov to k0001
Status: reopenednew

comment:13 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [7084]) Fixed #5124 -- Added a reasonable error when "extends" is not the first template tag. Patch from k0001.

Refs #6274.

Changed 9 years ago by Wayne K. Werner

Attachment: contains_nontext.diff added

comment:14 Changed 9 years ago by Wayne K. Werner

Has patch: set
Resolution: fixed
Status: closedreopened

In 7089, a 'contains_nontext' attribute was added to the NodeList class as a class attribute, it needs to be an attribute of a NodeList instance.

comment:15 Changed 9 years ago by Wayne K. Werner

Whoops! Sorry, I knocked that first diff out without thinking, of course when I ran my server, it failed. The correct diff is attached as contains_nontext_2.diff. Again, my appologies.

Changed 9 years ago by Wayne K. Werner

Attachment: contains_nontext_2.diff added

comment:16 Changed 9 years ago by Chris Beaven

Resolution: fixed
Status: reopenedclosed

Thanks AmanKow for the patch, but in the future, please open a new ticket for a new bug rather than re-opening this ticket (you can always reference it). In fact, I'll close this one again -- could you please open a new ticket for this and reattach your patch to that one?

Also, you haven't actually explained the problem. It may help to add some tests showing the failing behavior.

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