Skip to content

Commit 2b355c9

Browse files
authoredJan 29, 2021
Markdown: Workaround for incorrect highlighting due to double wrap hook (#2719)
The hook that highlights code blocks in markdown code was unable to handle code blocks that were highlighted already. The hook can now handle any existing markup in markdown code blocks.
1 parent 30b0444 commit 2b355c9

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed
 

‎components/prism-markdown.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,10 @@
347347
});
348348
}
349349
} else {
350-
// reverse Prism.util.encode
351-
var code = env.content.replace(/&lt;/g, '<').replace(/&amp;/g, '&');
350+
// get the textContent of the given env HTML
351+
var tempContainer = document.createElement('div');
352+
tempContainer.innerHTML = env.content;
353+
var code = tempContainer.textContent;
352354

353355
env.content = Prism.highlight(code, grammar, codeLang);
354356
}

‎components/prism-markdown.min.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

10 commit comments

Comments
 (10)

LeaVerou commented on Jul 2, 2021

@LeaVerou
Member

I've actually ran into issues of double highlighting with regular Prism too and had to employ hacks to heuristically detect if highlighting had already ran. I wonder if this is something we need to fix more centrally.

RunDevelopment commented on Jul 2, 2021

@RunDevelopment
MemberAuthor

I wonder if this is something we need to fix more centrally.

We certainly do.

The underlying problem here is that hooks aren't removed when the language gets reloaded. This leads to the same hook being executed multiple times.

To fix this, we have to figure out how to properly unload plugins, a long-standing issue (#459).

RunDevelopment commented on Jul 2, 2021

@RunDevelopment
MemberAuthor

I think our best shot for solving #459 will be the new dependency system (#2880) planned for v2.0. With some relatively minor additions, we can include hooks into that system. This gives us enough information to let Prism load and unload hooks automatically.

LeaVerou commented on Jul 2, 2021

@LeaVerou
Member

This problem is present even when no language gets reloaded, just with simple stock Prism included via a script tag, if you just pass it the same element to highlight twice.

RunDevelopment commented on Jul 2, 2021

@RunDevelopment
MemberAuthor

Oh, that's what you meant. You had an issue with Keep Markup, right? Yeah, that definitely a problem too.

mAAdhaTTah commented on Jul 2, 2021

@mAAdhaTTah
Member

Once we start on v2 & drop IE11 support, we could hold onto already-highlighted nodes in a Set or WeakSet so we can tag them as already-highlighted.

RunDevelopment commented on Jul 2, 2021

@RunDevelopment
MemberAuthor

IE11 does support most of Set and WeakMap. So whatever solution you have in mind, we could probably do it right now.

LeaVerou commented on Jul 2, 2021

@LeaVerou
Member

That wouldn't work, because we still want to be able to overwrite a node's contents and highlight it again.

mAAdhaTTah commented on Jul 2, 2021

@mAAdhaTTah
Member

I'm not 100% clear on the larger problem, but I was just thinking Set/WeakSet to solve the more direct problem, tracking whether highlighting already ran. The plugins that have problems when reloading can track to avoid re-highlighting, if that's something we want to do.

RunDevelopment commented on Jul 2, 2021

@RunDevelopment
MemberAuthor

The plugins that have problems when reloading can track to avoid re-highlighting, if that's something we want to do.

That's all plugins. This is a fundamental problem with the way we reload components. I don't think tracking already highlighted elements will help.

Please sign in to comment.