Skip to content

Multiple embedded files can break rendering #791

Open
@megaflo

Description

@megaflo

Under certain circumstances the rendering of pages breaks in the presence of multiple embedded elements. As far as I can tell this is related to wrong assumptions in the preprocessing of the markdown tokens.

Disclaimer: My JavaScript is not really great, so my analysis below could be off and is most definitely incomplete.

This could be related to #718 and #710 .

Short version

The condition

if (++count >= step) {

in line 46 of embed.js works under the assumption that the loop has finished before any of the constructed callbacks are called. This is not true under several conditions (for example due to the caching behavior in ajax.js), which results in broken rendering for pages with embedded elements.

This can be fixed by replacing the condition:

if (++count >= embedTokens.length) {

Long version

Problem description

In my concrete example I set up two separate pages with several embedded code fragments, with fragments from one particular code file being presented on both. When rendered individually (i.e. after a reload) they look fine. However, when switching to the other page e.g. via the menu that page would contain now other elements but some of the fragments (no text, headings, etc.).

Interestingly, this behaviour is symmetrical: reloading one page and switching to the other breaks rendering of the second one.

Possible cause

My investigation lead me to the preprocessing for the embedded elements in embed.js. As far as I can tell the condition in line 46

if (++count >= step) {

is meant to check, if we've handled all elements. The assumption seems to be, that the constructed callbacks will only be called after the loop has ended and step is equal to the number of tokens. The assumption checks out if the callback is scheduled/queued (for example as part of an AJAX call). However, due to caching in ajax.js for some elements the callback is actually not scheduled but executed immediately, triggering the condition early. This happened in my particular case, with the same code file being fetched on two different pages.

I think this particular bug also affects other types of embedded elements, as for all of these the callback is never scheduled but called immediately.

Sadly, my grasp on the project structure is not good enough yet to understand how signalling the end of the iteration early is producing the wrong results.

Possible fix

In a local copy I was able to fix the issue by modifying the condition in line 46 of embed.js. Instead of comparing count with step (relying on the loop being completed before any of the callbacks are called) we could just compare it with embedTokens.length:

if (++count >= embedTokens.length) {

I don't mind preparing a PR for this, but since I don't fully trust my understanding of the code, I would appreciate a second opinion on this first.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions