Opened 16 years ago

Closed 13 years ago

#5831 closed Bug (fixed)

Template Debug highlights wrong {% for %} tag

Reported by: Charmless <jim-django@…> Owned by: Vladimir Moskva
Component: Template system Version: dev
Severity: Normal Keywords: debug for toplevel nesting
Cc: m@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When an exception happens within a for tag's head, the error source is assigned always assigned to the top-level for loop, even if the erroneous for tag is nested.

Example:

{% for a in as %}
  {% for b in a.bs %}
    {% for c in b.relatedobject_set %}
       {{ c.foo }}
    {% endfor %}
  {% endfor %}
{% endfor %}

The error here is at "b.relatedobject_set", which should be "b.relatedobject_set.all" (assuming b is a model related to a model releatedobject).

As is, the template debug system marks the top level for tag, "{% for a in as %}", as being the point of error. The correct place to mark is the for tag where the error happened.

Attached is a patch that fixes this. I'm not familiar with Django's tag tests, so I can't include any, but would be happy to learn and supply some if it is agreed that this is a correct fix.

Attachments (2)

forloop.nodelist.patch (1.6 KB ) - added by Charmless <jim-django@…> 16 years ago.
patch against SVN revision 6626
forloop.nodelist.2.patch (5.2 KB ) - added by Vladimir Moskva 13 years ago.
Fixed typo in a comment

Download all attachments as: .zip

Change History (16)

by Charmless <jim-django@…>, 16 years ago

Attachment: forloop.nodelist.patch added

patch against SVN revision 6626

comment:1 by Charmless <jim-django@…>, 16 years ago

Summary: Template Debug only highlights wrong {% for %} tagTemplate Debug highlights wrong {% for %} tag

comment:2 by Gary Wilson, 16 years ago

#6468 marked as a dup. It also has a template example that shows the bug.

comment:3 by Gary Wilson, 16 years ago

Triage Stage: UnreviewedAccepted

comment:4 by charmless, 16 years ago

Owner: changed from nobody to charmless
Status: newassigned

comment:6 by Karen Tracey, 15 years ago

Resolution: fixed
Status: closedreopened

comment:7 by Gabriel Hurley, 13 years ago

Severity: Normal
Type: Bug

comment:8 by Julien Phalip, 13 years ago

Easy pickings: unset
Needs tests: set
Patch needs improvement: set

Confirming that this issue is still occurring. Needs tests to take this further. The patch also needs improvement as the ForNode's code has changed since 4 years ago.

comment:9 by Vladimir Moskva, 13 years ago

Owner: changed from charmless to Vladimir Moskva
Status: reopenednew
UI/UX: unset

comment:10 by Vladimir Moskva, 13 years ago

Needs tests: unset
Patch needs improvement: unset

django.template.defaulttags.ForNode.render method calls .render method for subnodes and there loses the information which exactly node caused the error, that's why DebugNodeList always marked the first forloop of nested as which caused the error.

I've made a patch which adds source of the the node that actually caused the error as the exception's attribute, which is later intercepted by DebugNodeList.render method.

Tests are included.

comment:11 by Vladimir Moskva, 13 years ago

Chanded order if and try..except statements. Now try..except is performed only when settings.TEMPLATE_DEBUG is true, so it shouldn't slower rendering with debugger disabled.

comment:12 by Vladimir Moskva, 13 years ago

Chanded order if and try..except statements. Now try..except is performed only when settings.TEMPLATE_DEBUG is true, so it shouldn't slower rendering with debugger disabled.

by Vladimir Moskva, 13 years ago

Attachment: forloop.nodelist.2.patch added

Fixed typo in a comment

comment:13 by Vladimir Moskva, 13 years ago

Cc: m@… added

comment:14 by Łukasz Rekucki, 13 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: newclosed

In [16605]:

Fixed #5831 -- Made sure the ForNode reports the correct source of an exception happening in one of the loops. Thanks, Charmless and vladmos.

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