Highlight.js and <details>

[AlexDawsonUK]

Description of problem

When using progressive disclosure using the details element in a ReSpec document, Highlight.js ceases to function and erases the content from the rendering tree.

URL to affected spec or repo:

https://w3c.github.io/sustyweb/drafts/ (Note as a temporary fix we're using nohighlight as a workaround.)

What happened (e.g., it crashed)?:

The code inside the pre element is erased and an empty tag is rendered instead of syntax being highlighted.

Expected behavior (e.g., it shouldn't crash):

The syntax should be highlighted, as it is for any code that would exist outside of the details element.

Optional, steps to reproduce:

1. Create a ReSpec draft document.
2. Add a details element with some syntax inside.

Show / Hide content.

!function(e,t){"use strict";"object"==typeof module&&"object"==typeof module.exports?module.exports=e.document?t(e,!0):function(e){if(!e.document)throw new Error("jQuery requires a window with a document");return t(e)}:t(e)}("undefined"!=typeof window?window:this,function(g,e){"use strict";var t=[],r=Object.getPrototypeOf,s=t.slice,v=t.flat?function(e){return t.flat.call(e)}:function(e){return t.concat.apply([],e)},u=t.push,i=t.indexOf

• This test showcases a basic ReSpec with it non-functional test.txt
• This test showcases a basic HTML file with it functional test2.txt

Note: I have verified that this issue does not occur in a normal HTML document using highlight.js therefore it possibly is due to ReSpec or its implementation of highlight?

[sidvishnoi]

@AlexDawsonUK I cannot reproduce the error with test.txt you shared.

Please check there's no < like unescaped HTML tags in your <pre> content. Also, please confirm if it works fine without a parent <details>.

[AlexDawsonUK]

Using the test.txt (As an HTML file), I am able to reproduce the error, though upon closer examination, the issue apparently seems to be isolated to Google Chrome. Using Mac build 124.0.6367.62 (latest). Unsure if it could be therefore a browser issue (as its only occurring in that browser) or if its a ReSpec issue (as its only an issue when using ReSpec). Unsure how to proceed with this, in any case I've submitted a report with Google as this might be a wider issue.

[sidvishnoi]

Thanks @AlexDawsonUK. I'll check in Chrome too. Could possibly be a ReSpec issue.

Edit: Confirmed happening in Chrome

[sidvishnoi]

Doesn't happen with <details open> in Chrome 🤓

[AlexDawsonUK]

Yes, I noted that odd quirk of behavior, and I even tried writing some JavaScript to close all the details components once everything had finished loading (hoping that it would give Highlight enough time todo its thing and then close them until required). But alas, whatever is causing the bug wouldn't have any of it. 🤷🏻‍♂️

I've expanded the testing and can confirm that all chromium browsers are affected (I tested in Chrome / Edge / Brave).
Firefox and Safari are both free from the issue though so I guess we can blame Google for a weird blink implementation of details? 😈

[sidvishnoi]

Agrees on blaming Blink behavior, as Safari and Firefox work fine. The actual issue is Blink returns empty string with innerText when content is not visible. I think ReSpec could use textContent, but I'm concerned about breaking whitespace.
https://github.com/w3c/respec/blob/fca26bdd681a2f3ecc3ff769375f09472fed439b/src/core/highlight.js#L26

cc @marcoscaceres Any idea why did we use innerText? Changed in https://github.com/w3c/respec/pull/2132.

[sidvishnoi]

Spec for reference: https://html.spec.whatwg.org/multipage/dom.html#get-the-text-steps

[AlexDawsonUK]

Update: I've created a potential workaround patch that allows functioning rendering engines to continue to enjoy syntax highlighting while chromium functions with it disabled. It requires the use of the nohighlight class on all affected code.

<script async>
	const isFirefox = navigator.userAgent.toLowerCase().includes('firefox');
	const isSafari = navigator.vendor && navigator.vendor.indexOf('Apple') > -1 &&
             navigator.userAgent &&
             navigator.userAgent.indexOf('CriOS') == -1 &&
             navigator.userAgent.indexOf('FxiOS') == -1;
	if (isFirefox || isSafari) {
		for (let i = 0; i < document.getElementsByTagName('pre').length; i++) {
			if (document.getElementsByTagName('pre')[i].className = "nohighlight" ) {
				document.getElementsByTagName('pre')[i].className = ""
			}
		}
	}
</script>

It requires highlighting to be enabled on functional browsers rather than disabling it on non-functional ones due to the issue of the loss of content being displayed from the rendering tree as mentioned previously regarding innerText.

[marcoscaceres]

I can't recall, but probably because .textContent is wonky. We could try it though.

[nigelmegitt]

if (document.getElementsByTagName('pre')[i].className = "nohighlight" )
document.getElementsByTagName('pre')[i].className = ""

You probably want:

document.getElementsByTagName('pre')[i].classList.remove("nohighlight");