Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The line numbering and the syntax highlighter doesn't work properly. #34

Open
sahilrl opened this issue Dec 22, 2022 · 14 comments
Open

Comments

@sahilrl
Copy link

sahilrl commented Dec 22, 2022

When I reload the website sometimes the syntax highlighting and the numbering doesn't appear. If I keep reloading the webpage multiple times, then it appears after several tries. And if I reload it one more time, then the highlighting and numbering disappear again.

It's an issue only encountered when rendering on templates, not the admin panel.

Appearing now
Screenshot 2022-12-22 at 07-37-24 Coffee Spiller

on next reload, disappearing.
Screenshot 2022-12-22 at 07-38-37 Coffee Spiller

@FlipperPA
Copy link
Owner

@sahilrl Have you looked at the JavaScript console of your browser while loading the page, to make sure all the proper assets are loading? That'd be the first place to check.

@sahilrl
Copy link
Author

sahilrl commented Jan 21, 2023

Hi, @FlipperPA I just checked and in the HTML, the <span> on the code sometimes doesn't show up after reloading the page.

The problem is occurring even when the following files are loading.

JS
https://cdnjs.cloudflare.com/ajax/libs/prism/1.28.0/components/prism-python.min.js
https://cdnjs.cloudflare.com/ajax/libs/prism/1.28.0/prism.min.js

CSS
https://cdnjs.cloudflare.com/ajax/libs/prism/1.28.0/themes/prism-okaidia.min.css
https://cdnjs.cloudflare.com/ajax/libs/prism/1.28.0/plugins/toolbar/prism-toolbar.min.css

All the spans on the code are sometimes missed on reload.

<span class="token keyword">from</span> ... <span class="token punctuation">.</span> ... <span class="token class-name">BlogPageTag</span>

@FlipperPA
Copy link
Owner

@sahilrl Just to clarify, you're saying the SPANs are in the HTML source, but they're not being highlighted? Is there anything in the JavaScript console like an error that might help us narrow it down? The loading mechanism used on the template side is kind of funky; since multiple code blocks can appear on a page, it uses JavaScript to load JavaScript libraries necessary, only once. I'm wondering if that's where it is getting hung up.

@sahilrl
Copy link
Author

sahilrl commented Jan 23, 2023

@FlipperPA No, actually the spans are not appearing at all.

This is with spans
Screenshot from 2023-01-23 22-47-37

And sometimes, the webpage load without rendering spans.
Screenshot from 2023-01-23 22-48-18

@cesaregarza
Copy link

cesaregarza commented May 11, 2023

Can confirm this issue happens at random. Only console error is https://cdnjs.cloudflare.com/ajax/libs/prism/1.29.0/plugins/copy-to-clipboard/prism-copy-to-clipboard.min.css getting a 404, but this happens regardless of whether or not the line numbers load.

@frankcorneliusmartin
Copy link

frankcorneliusmartin commented Sep 21, 2023

I also encounter the same thing as @sahilrl . It only seems to happen with the Python syntax.. Any workarounds?

@FlipperPA
Copy link
Owner

It sounds like some kind of race condition may be going on, and because of how I'm dynamically loading libraries... it is tough to debug. I'm due to upgrade the underlying version of PrismJS as well. Let me do that and see if it improves things at all, because these kinds of issues are so hard to reproduce.

@frankcorneliusmartin
Copy link

Thanks you for the quick reply. I also observe that in our live version it occurs less than in my local testing environment supporting your suspicion that this is a racing condition.

@FlipperPA
Copy link
Owner

@frankcorneliusmartin @sahilrl @cesaregarza I'm wondering if any of you have a solid reproduction for this that you could pass along to me. We use v1.29.0.0 very heavily at my workplace, and we haven't had any reports of issues like this. If you could set up a git repo with a minimal repro, that would be amazing.

Also, what version of Wagtail and wagtailcodeblock are you running? It's possible this could be an upstream effect from a newer / older version of Wagtail.

@sahilrl
Copy link
Author

sahilrl commented Oct 23, 2023

Hello @FlipperPA,

wagtail==5.1.3
wagtailcodeblock==1.29.0.1
django==4.2.6

To reproduce:

Step1: Clone repository git clone [email protected]:sahilrl/reproduce_bug_wagtailcodeblock.git

Step2: Create virtual environment python -m venv venv

Step3: Install dependencies python install -r requirements.txt

Step4: Run server python manage.py runserver

Step5: In the browser, navigate to localhost:8000

Refresh multiple times and notice that at random, the syntax highlighting doesn't work.

@FlipperPA
Copy link
Owner

@sahilrl Thank you! I'll take a look into this.

Is everyone else using Wagtail 5.x? My workplace is still on 4.x, wondering if that's the problem.

@lb-
Copy link

lb- commented Oct 26, 2023

@FlipperPA my best guess is that the lines plugin is being parsed by the browser just before the actual core library in some cases.

Here's a starting point for a refined script that will wait for the core library to be loaded and only then will it try to load the other plugins.

See load event https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/load_event

{% load static wagtailcodeblock_tags %}
...
            if(typeof loadPrismLanguage != 'function') {
                window.loadPrismLanguage = function(language) {
                    var libraries = [
                        /*Since there is no html.min.js this check makes sure we
                        bypass the loading of the script when syntax is set to HTML */
                        {% if val != "html" %}
                            ,
                            {
                                "id": "code-block-prismjs-" + language,
                                "url": "//cdnjs.cloudflare.com/ajax/libs/prism/{% prism_version %}/components/prism-" + language + ".min.js"
                            }
                        {% endif %}
                        {% line_numbers_js %}
                        {% toolbar_js %}
                        {% copy_to_clipboard_js %}
                    ];


                    var core = {
                        "id": "code-block-prismjs",
                        "url": "//cdnjs.cloudflare.com/ajax/libs/prism/{% prism_version %}/prism.min.js"
                    };
                    // here we create a promise that will resolve immediately if the element is in the DOM
                    // or it will wait until the script is loaded before the rest of the plugins run
                    // another approach may be to do a setTimeout (with no delay) IF the script is in the DOM already - this way we know we will not try to run the other libraries until the current JS has run.   
                    new Promise((resolve) => {
                        if (document.getElementById(core['id'])) resolve();
                        var s = document.createElement("script");
                        s.id = core["id"];
                        s.type = "text/javascript";
                        s.src = core["url"];
                        s.async = false;
                        s.addEventListener('load', resolve, {once: true});
                    }).then(() => {
                        for(const library of libraries) {
                            if(document.getElementById(library["id"]) == null) {
                                var s = document.createElement("script");
                                s.id = library["id"];
                                s.type = "text/javascript";
                                s.src = library["url"];
                                s.async = false;
                                document.body.appendChild(s);
                            }
                       }
                    });
                };
            }

            loadPrismLanguage('{{ val }}');

            language_class_name = 'language-{{ val }}';
            </script>

However, a long term solution would probably be to provide a single template tag that needs to be included in pages that may contain the codeblock.

This could still async load the JS as needed, maybe seeing if a DOM element appears in the dom with the mutation observer or using Wagtail 5.2's support for Stimulus ;)

This central script would bootstrap itself once any code block with a specific data attribute appears and then use classes/data attributes to determine what other libraries should load as needed.

@frankcorneliusmartin
Copy link

@sahilrl Thank you! I'll take a look into this.

Is everyone else using Wagtail 5.x? My workplace is still on 4.x, wondering if that's the problem.

Yes indeed, we are on 5.

@cesaregarza
Copy link

Also on 5.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants