Automatic code highlighting broken in fullscreen code view

AI-generated summary

A bug was reported by oskar1 where automatic code highlighting in Discourse fails in fullscreen code view, despite working in normal topic view. pmusaraj confirmed the bug and mentioned that a fix would be looked into soon. Arkshine identified the potential issue in the highlight-syntax.js file and proposed a fix. With pmusaraj’s encouragement, Arkshine created a pull request (#26710) with the proposed fix, which pmusaraj reviewed and approved pending successful tests.

Priority/Severity:
Low
Platform:
Chrome 124, Ubuntu 22
Version:
Discourse v3.3.0.beta2-dev
Description:
Discourse performs automatic code highlighting with hljs, even when no language was specified. This feature works in normal topic view, but not when viewing the code in fullscreen.
Reproducible steps:

  1. Create a topic with a code block, without specifying the language
System.out.println("hello world");
  1. View the topic and see that hljs detected a language and applies highlighting.
  2. Click the fullscreen button at the top right of the code block.
  3. See that hljs language detection failed and no highlighting is applied

Here are some screenshots in one big image, because of new user restrictions :confused:.

3 Likes

Thanks for the report, @oskar1, I can confirm this bug locally. (It’s harder to reproduce this on meta because we use text not auto as the default code lang on meta.)

We will take a look at fixing this soon.

3 Likes

I think the issue is here :thinking: :

https://github.com/discourse/discourse/blob/main/app/assets/javascripts/discourse/app/lib/highlight-syntax.js#L36-L52

You have lang-auto class name applied with markdown code and automatically removed here so the library can guess the language.

However, when you go fullscreen, lang-auto is no longer in the element, and you have only language-<detected-language-here> class name here. Thus, lang is undefined.

A maybe fix could be here – if lang-auto is not found, look for language-<..>

const m = className.match(/^lang-(.+)$/);
→
const m = className.match(/^`(?:lang|language)-(.+)$/);

1 Like

Happy to review a PR if you’d like to submit one @Arkshine, you’ve done a good part of the work already :wink:.

3 Likes

I made a PR :+1: . Hopefully, the tests are good enough!

https://github.com/discourse/discourse/pull/26710

2 Likes

Thanks @Arkshine, fix looks great, will merge it once tests pass in CI.

3 Likes

This topic was automatically closed after 18 hours. New replies are no longer allowed.