Plugins incompatibility: File Highlight and Copy to Clipboard

[majal]

Information:

• Prism version: [1.16.0]
• Environment: [Browser]

Does the latest version of Prism from the download page also have this issue? Yes.

Description
The plugin Copy to Clipboard works well if the code is sourced between the <pre><code> tags. On the other hand, if the code was sourced using the File Highlight plugin, e.g.

<pre data-src="/path/to/asset" class="language-shell" data-download-link="" data-download-link-label="download"></pre>

... then Copy to Clipboard fails. Instead of copying the code, it simply copies Loading…, which interestingly, is what initially appears when File Highlight is still loading the code from the file.

Example
Here is a sample webpage that shows this incompatibility: https://www.majlovesreg.one/adding-brotli-to-a-built-nginx-instance.

[RunDevelopment]

Alright, after a few hours of debugging your website, I finally found the answer! ^_Yaaaaay

TLDR;

Don't load the script containing Prism asynchronously until this is fixed.

The long version

File Highlight and Copy to Clipboard are not incompatible. In fact, the plugin page of Copy to Clipboard uses File Highlight to load the code which is then can then be copied.
(You can't see the copy button because of an issue of Toolbar and the Prism page design (will be fixed soon). In the meantime, setting right: 10em for div.code-toolbar > .toolbar in the dev tools of your browser will make the button visible.)

The problem is that File Highlight has to executed after the initial Prism.highlightAll because the FH will insert a code element with the text Loading... inside all pres with a data-src attribute. If FH were to be executed before Prism.highlightAll, all of the Loading...-code-elements would be highlighted as well (which is not intended) only to highlighted again after the code has been loaded.
One of the consequences of this double-highlighting is that all plugins are run twice on the code element. Most plugins check for multiple executions on the same element, so this usually isn't a problem but it is in this case because the text content of the code elements changed from Loading... to whatever code has been loaded. This is the reason why the copied text was Loading...: The Copy to Clipboard plugin also detects these multiple executions on the same element.

Then why was File Highlight executed before Prism.highlightAll in the first place?
Events. This is the code which adds FH's callback and this adds the Prism.highlightAll callback.
If the Prism script is loaded synchronously, both FH and highlightAll use the DOMContentLoaded event. Because Prism Core is always loaded before FH, the FH listener will be executed after the highlightAll listener, so no problem.
But if the Prism script is loaded asynchronously, the DOMContentLoaded event might have already been fired, so we use requestAnimationFrame (or setTimeout) instead. The problem here is that just because document.readyState !== "loading" doesn't mean that the DOMContentLoaded event has been fired because there is the interactive ready state. So what happens is that highlightAll uses requestAnimationFrame and gets executed before FH which uses DOMContentLoaded.

Depending on how you look at it, the bug is either that File Highlight doesn't use the same async logic as Prism Core or that the async logic of Core doesn't quite work.

Thoughts? @mAAdhaTTah @Golmote.

Btw. #959 added Prism Core's async logic.

[RunDevelopment]

Interestingly, your website works flawlessly in the best of all browsers: IE11.

[majal]

Thanks for spending time on this. I'm sure the devs will nail this one soon. :-) Prism is a great project. I love it.

Now for this:

Interestingly, your website works flawlessly in the best of all browsers: IE11.

It took me a few minutes to explain this to my wife. :-D

[majal]

Your comment on IE 11 yesterday caused me to further test here at work where we use Windows 10 Pro (v. 10.0.17134). Here are the results on browsers on Windows 10:

Google Chrome v. 75.0.3770.100 - it works!
IE v. 11.829.17134.0 - nothing registers to the clipboard, like copy never happened.
Edge v. 42.17134.1.0 - it works!

I also re-tested on Ubuntu 18.04:

Google Chrome v. 75.0.3770.100 - "Loading..." on clipboard.
Firefox Quantum v. 67.0.4 - "Loading..." on clipboard.

What is interesting is we apparently get somewhat different results... Hmmm...

[RunDevelopment]

Interesting results.
I don't understand why Chrome gives you different results depending on the OS but from my understanding, we're moving the gray area of the spec so maybe that's to be expected?

Another reason for these results is likely to be caching. The cached async script will load almost instantaneously which might fast enough so that everything is loaded before the next frame is rendered which will cause DOMContentLoaded to be executed before requestAnimationFrame. If that guess is correct then the behavior of your website will depend not only on whether the script is cached but also on the cache type/implementation and on the hardware of the system you're running.
Truly an interesting race condition we have on hand.

[mAAdhaTTah]

Depending on how you look at it, the bug is either that File Highlight doesn't use the same async logic as Prism Core or that the async logic of Core doesn't quite work.

To clarify, it's not the "async" logic but the initialization logic, yeah? I ask cuz Prism can run async using a Worker, but I don't think, based on your explanation, that's what happening here. Is that correct?

[RunDevelopment]

With async I mean <script src="prism.js" async></script>.

And yes with async logic I mean this initialization logic.

[mAAdhaTTah]

Yeah... I've never been in love with that logic, even though I wrote some of it¹. That said, looking at the file highlight plugin, it does feel like it should hook into Prism, rather than doing its own highlighting and then using Prism. Like control here seems inverted from how it should work.

I think we have the hooks to actually make this feasible. We can use before-highlightall to modify the selector to include the pre[data-src] query FH uses. Looking at the logic, it'll probably hit this block, so we could use the complete hook to trigger the current highlighting logic, which would run it again through the usual flow.

Thoughts?

Footnotes

1. Edit: At least, I think I did. I'm not in the git blame tho, so maybe not? I thought I did something related to manual back in the day... ↩

[RunDevelopment]

We can use before-highlightall to modify the selector to include the pre[data-src] query FH uses.

I really like this idea. A few of my thoughts on this in no particular order.

• The callback function should be executed after the file has been loaded and highlighted and not in the "first run". In this case, the inconsistent callback handling might actually come in handy (here the callback isn't called Edit: fixed in Core: Minor improvements #1973).
• Regardless of what we do, we will always execute the before-sanity-check and complete hooks. This might cause problems with other plugins using these hooks as most plugins simply assume that env.element is a <code> element. This is also a criticism of these plugins as the unescaped markup plugin exists for quite some time now.
• What about backward-compatibility? FH exposes its main function via Prism.fileHighlight (really should be Prism.plugins.fileHighlight). Maybe just log a deprecation message (or similar) and remove it some time later?
• We should also do the same for JSONP Highlighting.