This application is not used any more and exists only for history. Post new tickets on the Github account.
Cette application n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Ticket #1396 (closed bug: fixed)

Opened 3 years ago

Last modified 3 years ago

jtpl plugin "include" can not handle recursivity because of metas

Reported by: bricet Owned by:
Priority: normal Milestone: Jelix 1.2.4
Component: jelix:plugins:tpl Version: 1.2.3
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Since #1196 , jTpl plugin "include" can not be called on the template where it is called because {meta} is called recursively, with no end condition.

vincent_v spotted that bug

Attachments

patch_1396.diff (2.0 KB) - added by bricet 3 years ago.
patch_1396_bis.diff (2.7 KB) - added by bricet 3 years ago.

Change History

comment:1 Changed 3 years ago by laurentj

I really don't understand what you mean. Could you explain please?

comment:2 Changed 3 years ago by bricet

I admit the explanation was quite cryptographic so that I could just remember myself what the problem is ...

Let's assume (not tested by myself for now, but this seems to be the idea) we have a template a.tpl :

{=count($items)} items
{assign $dummy=array_pop($items)}
{if count($items) > 0}
{include 'a'}
{/if}

If you fetch a with any non-empty array as a content, you should expect to things to be ok.

But we have troubles since #1196 because meta on this tpl are called recusively and end conditions are not checked for metas. So we lead to an infinite call stack.

One more time : I did not tested it myself and will as soon as I can. If this test case does not reveal the bug and/or I misunderstood how to trigger that bug, we should ask vincent_v.

comment:3 Changed 3 years ago by bricet

  • review set to review?

Test case approved.

Here is a bugfix and a unit test.

Changed 3 years ago by bricet

comment:4 Changed 3 years ago by laurentj

It seems that a template is missing in the patch: 'test_plugin_include_recursive_included'. isn't it?

comment:5 Changed 3 years ago by bricet

Indeed I made a mistake (one more time).

I don't have my repo right now, but I suppose we should just replace test_plugin_include_recursive_included by test_plugin_include_recursive in test_include_recursive.tpl

comment:6 Changed 3 years ago by laurentj

  • review changed from review? to review-
  • Milestone set to Jelix 1.2.4

Ok. The problem with your test, is that it doesn't test with meta tag, whereas it is the original issue ;-) Your template should content meta instructions, and test if meta (retrieved with the corresponding method of jTpl) are ok.

comment:7 Changed 3 years ago by bricet

The use of meta is not the issue.

Try the unit test without the patch and you will see (well you won't see anything, actually) ;)

We could do a unit test with meta too, expecting to get metas of only first "call" to the template, not recursive calls.

The patch solves troubles in all cases : we can use {include} recursively. And we still have some benefits of #1196 since we get metas of first "call" of template (there is already a unit test for that).

comment:8 Changed 3 years ago by bricet

  • review changed from review- to review+

Ok here is a unit test changed so that we check metas too.

The expected result is to get the meta of the first "call" of the template. This is a choice and this is mine. Technically speeking, I can not see for now how to behave another way (other than first call's value). Therically speeking, it seems to me this choice is not so bad and will meet most requirements (include js, css, ...).

Don't know if that was clear before, but before this patch (and after patch provided by #1196), it was not possible to {include} a tppl recursively, even if he had no metas.

Changed 3 years ago by bricet

comment:9 Changed 3 years ago by bricet

  • review changed from review+ to review?

comment:10 Changed 3 years ago by laurentj

  • Status changed from new to closed
  • review review? deleted
  • Resolution set to fixed

this patch does not fix completely the issue. If we have two {include} in a same template, we will have two static declaration of the same variable in the meta content.

Furthermore, if we have a template A which include a template B which include itself, meta of template B will be processed twice, because the generated test in the meta function of A doesn't share the same static variable of the generated test in the meta function of B.

So we have to use an other strategy. The jTpl object should have the role to verify this case. I made a patch. https://bitbucket.org/jelix/jelix-1.2.x/changeset/0eeb5889fe9a

It should be tested on "real" web site to verify that it doesn't introduce regression. Reopen the bug if necessary.

Note: See TracTickets for help on using tickets.