Multiple embedded files can break rendering

[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.

[marisk]

I have same issue with multiple embeds. I'm using ~30 .md includes in main README.md (one include per one documented API method) and rendering docsify in browser with cold cache (or cache disabled) almost always results in randomly missing includes or messing up order of includes. Looks like main .md file gets rendered before all includes are loaded.

[davidtabernerom]

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.

Just tested it, it does not work, sorry, still randomly sorting embedded files.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.