Opened 8 years ago

Closed 7 years ago

#7501 closed (fixed)

template rendering doesn't reset cycle tag

Reported by: mitsuhiko Owned by: nobody
Component: Template system Version: master
Severity: Keywords:
Cc: elsdoerfer@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


The cycle iter is stored on the node. Unless templates are re-parsed each request that will mean that multiple template renderings share the same cycle iter. As a result of that it is very likely that cycle skips items on renderings.

Proposed fix: store the iter on the context object.

Attachments (1)

fixcyclebehaviour.patch (2.5 KB) - added by steveire 7 years ago.
Fixes cycle reset issue by adding reset feature to node.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by anonymous

  • Cc elsdoerfer@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 8 years ago by ericholscher

  • milestone changed from 1.0 to post-1.0
  • Summary changed from {% cycle %} is not threadsafe to template rendering doesn't reset cycle tag
  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 8 years ago by SmileyChris

In #5908 Malcolm talks about the importance of keeping the state of the internal counter. Currently, the {% for %} tag (and potentially others) sets up a local context for each iteration, so if we put the counter straight on the context, it'll be lost the next iteration.

I think Context needs a way to set a "global" context variable (if you understand the internals, probably just stuffing it at the highest level context).

comment:4 follow-up: Changed 8 years ago by harryman100

There's a workaround for this which can be used in templates:

Using the 'cycle as' syntax resets the variable. By outputting the 'cycle as' inside a comment it hides it from the rendering. It's not perfect, but it works in most cases

<!--{% cycle 'rowB' 'rowA' as rowstyle %}-->
{% for post in posts %}
        <tr class="{% cycle rowstyle %}">
{% endfor %}

comment:5 in reply to: ↑ 4 Changed 8 years ago by harryman100

Replying to harryman100:
Scrap that, it was fluke that it worked with a particular dataset

comment:6 Changed 8 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:7 Changed 7 years ago by kmtracey

#11290 reported this again.

comment:8 Changed 7 years ago by bpeschier

I came across this a few times when rendering multiple tables next to each other with the same cycle (with 2 arguments for colouring) on a row. A double loop. If a table had an even amount of rows, the colours matched, otherwise, a grid appeared. It came out ugly sometimes, but acceptable.

Looking for a quick fix, I suspected there must be a reset of some sort, but there is not.

The {% cycle [] as var %} could be a nice way to fix this. But for starters, the output of {% cycle [] as var %} should go away. "as var" in other tags denotes a variable for later use. If we can reference the cycle-variable on the node, we could make a {% cycle reset var %} of some sort, which would reset the cycle referenced.

comment:9 Changed 7 years ago by steveire

  • Has patch set

The attached patch fixes the issue and passes the test attached to #11290.

The test and fix are attached in the same patch here for completeness.

Changed 7 years ago by steveire

Fixes cycle reset issue by adding reset feature to node.

comment:10 Changed 7 years ago by SmileyChris

This would be fixed by #6262 - and I consider the ticket there to contain the correct fix to this issue.

comment:11 Changed 7 years ago by russellm

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

Resolved by [11862]

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