django-sekizai icon indicating copy to clipboard operation
django-sekizai copied to clipboard

helpers._scan_namespaces silently gives up when encountering a with_data node (SekizaiParser dropping template nodes?)

Open greyhare opened this issue 3 years ago • 9 comments

While trying to chase down a spurious check failure bug in Django CMS, I found what seems to be a bug in sekizai.helpers._scan_namespaces(nodelist, current_block). (CMS's cms check management command is using sekizai.helpers.validate_templates to ensure the presence of css and js render blocks.)

I stuffed this printout into the code:

def _scan_namespaces(nodelist, current_block=None):
    from sekizai.templatetags.sekizai_tags import RenderBlock
    found = []

    for node in nodelist:
        # check if this is RenderBlock node
        print(node)
        if isinstance(node, RenderBlock):

The template code that triggers the fault (from my base.html template's <head> section):

    {% include 'favicons.html' %}

    <!-- Latest compiled and minified Bootstrap 4.3.1 CSS -->
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
    {% with_data "google-fonts" as fonts %}{% if fonts %}
    <link href="https://fonts.googleapis.com/css?family={{ fonts|join:',' }}" rel="stylesheet">
    {% endif %}{% end_with_data %}
    <!-- Compressed CSS -->
    {% render_block "css" postprocessor "compressor.contrib.sekizai.compress" %}

The printouts corresponding to this part of the template are:

<django.template.loader_tags.IncludeNode object at 0x10f7f2ac0>
<TextNode: '\n\n    <!-- Latest compile'>
<Tag: with_data>

and there are no further nodes and I have no idea why.

Removing the {% with_data ... end_with_data %} block fixes the problem.

greyhare avatar Jan 28 '22 02:01 greyhare

To be clear, I expect the output should have been:

<django.template.loader_tags.IncludeNode object at 0x10f7f2ac0>
<TextNode: '\n\n    <!-- Latest compile'>
<Tag: with_data>
<TextNode: '\n    <!-- Compressed CSS '>
<Tag: render_block>

greyhare avatar Jan 28 '22 03:01 greyhare

Thanks for raising this @greyhare

I don't use post processors, but will endeavour to get visibility of this on our slack.

marksweb avatar Jan 28 '22 09:01 marksweb

Thanks for raising this @greyhare

I don't use post processors, but will endeavour to get visibility of this on our slack.

I have just confirmed that the exact same bug occurs when the postprocessor clause is removed:

    {% include 'favicons.html' %}

    <!-- Latest compiled and minified Bootstrap 4.3.1 CSS -->
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
    {% with_data "google-fonts" as fonts %}{% if fonts %}
    <link href="https://fonts.googleapis.com/css?family={{ fonts|join:',' }}" rel="stylesheet">
    {% endif %}{% end_with_data %}
    <!-- Compressed CSS -->
    {% render_block "css" %}

outputs:

<django.template.loader_tags.IncludeNode object at 0x10f7f2ac0>
<TextNode: '\n\n    <!-- Latest compile'>
<Tag: with_data>

greyhare avatar Jan 28 '22 12:01 greyhare

Digging further, the nodelist passed to _scan_namespaces ends with the with_data node, so the bug is eariler. Stuffing in a bunch of traces in the other nodes to see what's going on.

I'm wondering if the fact that with_data is a subclass of SekizaiTag while render_node is a subclass of Tag might be part of it. All SekizaiTag does different from Tag is refuse to render if there's no SEKIZAI_CONTENT_HOLDER in the context. Is that missing from the context when validate_templates is called from, say, a check management command?

greyhare avatar Jan 28 '22 15:01 greyhare

Digging further, the nodelist passed to _scan_namespaces ends with the with_data node, so the bug is eariler. Stuffing in a bunch of traces in the other nodes to see what's going on.

I've traced the short nodelist back to tpl.nodelist in _get_nodelist(tpl).

I'm wondering if the fact that with_data is a subclass of SekizaiTag while render_node is a subclass of Tag might be part of it.

Not it.

greyhare avatar Jan 29 '22 03:01 greyhare

In _extend_nodelist(extend_node) with extend_node as the "extends" node in my CMS page template, it gets down to

    parent_template = extend_node.get_parent(get_context())

Where parent_template is my base.html template file, which has the with_data block. The parent_template.template property in that object ends after the with_data node. So it looks like someting is making Django's template-file-to-nodes parser die early after a with_data node. But only here; when processed for real during a view, I see everything in the template working correctly.

I want to say this reminds me of some weirdness about properly processing custom template tags which have end tags, but that memory's from years ago and it's fuzzy.

greyhare avatar Jan 29 '22 03:01 greyhare

And here we are. I stuffed a printout into SekizaiParser.parse_blocks to print out what you guys are storing in self.blocks['nodelist'], and it's spitting out a series of lists (in reverse order!) that comprise the rest of my base.html template, starting with the block after the end_with_data tag. SekizaiParser being used by class WithData(SekizaiTag) which has options.parser_class=SekizaiParser.

I'm done. There are no comments, so I don't know why you're stuffing parser nodes into a new "block" named "nodelist", which I assume gets passed to the nodelist parameter of WithData.render_tag since nodelist isn't called out in options. But it looks like this sequestering results in my base.html template's template ending with the with_data tag.

I've attached a minimal set of template files which should invoke this bug: Archive.zip

Ball's in your court. I don't know enough about sekizai's design strategy for with_data, or why SekizaiParser is even needed, to go further.

greyhare avatar Jan 29 '22 04:01 greyhare

just a quick shot: did you notice https://django-sekizai.readthedocs.io/en/latest/#restrictions ?

as far as I can tell, your {% with xy=z %} tag containing a {% render_block %} is that scenario/restriction, no?

forgive me and ignore this if not - I'm not familiar with inner TemplateTag rendering code, and only understand half of the rest of your digging... ;-)

benzkji avatar Feb 12 '22 13:02 benzkji

This is the entire {% with_data %} block:

{% with_data "google-fonts" as fonts %}{% if fonts %}
<link href="https://fonts.googleapis.com/css?family={{ fonts|join:',' }}" rel="stylesheet">
{% endif %}{% end_with_data %}

There's no {% render_block %} in it. Further, none of this is inside another tagged block; it's in the <head> section of my base template.

I included the {% render_block %} that comes after the {% with_data %} block in my example because I expected to see it output in the node printouts, and it wasn't.

greyhare avatar Feb 12 '22 13:02 greyhare

Anyone have any ideas on this?

greyhare avatar Sep 29 '22 18:09 greyhare

Yeah, see #142 😀 Wanna test? Get the fix by pip install git+https://github.com/fsbraun/django-sekizai@master. Let me know if it works!

fsbraun avatar Sep 29 '22 20:09 fsbraun

Gonna be a bit before I can recreate the test scenario.

greyhare avatar Sep 29 '22 21:09 greyhare

Thanks for being persistent with this issue despite stalebot and the considerable time this has been open!

fsbraun avatar Sep 30 '22 04:09 fsbraun