Opened 9 years ago

Closed 5 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: master
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@…> 9 years ago.
patch against SVN revision 6626
forloop.nodelist.2.patch (5.2 KB) - added by Vladimir Moskva 5 years ago.
Fixed typo in a comment

Download all attachments as: .zip

Change History (16)

Changed 9 years ago by Charmless <jim-django@…>

Attachment: forloop.nodelist.patch added

patch against SVN revision 6626

comment:1 Changed 9 years ago by Charmless <jim-django@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Summary: Template Debug only highlights wrong {% for %} tagTemplate Debug highlights wrong {% for %} tag

comment:2 Changed 9 years ago by Gary Wilson

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

comment:3 Changed 9 years ago by Gary Wilson

Triage Stage: UnreviewedAccepted

comment:4 Changed 8 years ago by charmless

Owner: changed from nobody to charmless
Status: newassigned

comment:6 Changed 8 years ago by Karen Tracey

Resolution: fixed
Status: closedreopened

comment:7 Changed 5 years ago by Gabriel Hurley

Severity: Normal
Type: Bug

comment:8 Changed 5 years ago by Julien Phalip

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 Changed 5 years ago by Vladimir Moskva

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

comment:10 Changed 5 years ago by Vladimir Moskva

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 Changed 5 years ago by Vladimir Moskva

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 Changed 5 years ago by Vladimir Moskva

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.

Changed 5 years ago by Vladimir Moskva

Attachment: forloop.nodelist.2.patch added

Fixed typo in a comment

comment:13 Changed 5 years ago by Vladimir Moskva

Cc: m@… added

comment:14 Changed 5 years ago by Łukasz Rekucki

Triage Stage: AcceptedReady for checkin

comment:15 Changed 5 years ago by Jannis Leidel

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