Opened 8 years ago

Closed 7 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 7 years ago.
adds support for a new Node property "must_be_first"
contains_nontext.diff (665 bytes) - added by AmanKow 7 years ago.
contains_nontext_2.diff (675 bytes) - added by AmanKow 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by vdupras@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 8 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 8 years ago by ubernostrum

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

comment:4 Changed 7 years ago by anonymous

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:5 Changed 7 years ago by buriy

  • Owner changed from anonymous to buriy
  • Status changed from assigned to new

comment:6 Changed 7 years ago by buriy

  • Status changed from new to assigned

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 7 years ago by buriy

  • Triage Stage changed from Unreviewed to Design 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 7 years ago by buriy

  • Triage Stage changed from Design decision needed to Accepted

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 7 years ago by k0001

adds support for a new Node property "must_be_first"

comment:9 Changed 7 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 7 years ago by k0001

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:11 Changed 7 years ago by SmileyChris

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Accepted to Ready 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 7 years ago by buriy

  • Owner changed from buriy to k0001
  • Status changed from reopened to new

comment:13 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

Refs #6274.

Changed 7 years ago by AmanKow

comment:14 Changed 7 years ago by AmanKow

  • Has patch set
  • Resolution fixed deleted
  • Status changed from closed to reopened

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 7 years ago by AmanKow

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 7 years ago by AmanKow

comment:16 Changed 7 years ago by SmileyChris

  • Resolution set to fixed
  • Status changed from reopened to closed

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