Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax highlighting adds unwanted HTML code comments #2744

Closed
greg-witczak opened this issue Aug 1, 2020 · 11 comments
Closed

Syntax highlighting adds unwanted HTML code comments #2744

greg-witczak opened this issue Aug 1, 2020 · 11 comments

Comments

@greg-witczak
Copy link

Reveal code with php syntax highlight:
image

Renders like this:
image

Generated HTML code:

<pre><code class="php hljs">&lt;!--?php
<span class="hljs-function"><span class="hljs-keyword">function</span> <span class="hljs-title">myFunc</span>(<span class="hljs-params"></span>)</span>{
    <span class="hljs-keyword">echo</span> <span class="hljs-string">'test'</span>;
}
$name = <span class="hljs-string">'myFunc'</span>;
$name();&lt;/code--&gt;</code></pre>

I can't see any errors/warnings during build process.

This bug seems to be similar to #2741 and #2732 - reveal is adding something strange, trying to make code better. Is there a way to disable it?

@FrancoSpeziali
Copy link

I'm having a similar issue, when displaying React code. Seems to a problem in the parsing somewhere. Problem does not happen in older versions of reveal.js, so this is new for sure.

@topdeal2021

This comment has been minimized.

@greg-witczak
Copy link
Author

@FrancoSpeziali do you know the older version that is working fine? I'm considering downgrade to fix it.

@FrancoSpeziali
Copy link

@greg-witczak I haven't tried all the versions, but I checked out tag 3.7.0 and I could not reproduce the issue there. Possibly some later versions don't have the bug, I would investigate further, but time issues...!
It's possible the bug isn't in reveal.js, but one of it's dependencies

@hakimel
Copy link
Owner

hakimel commented Sep 9, 2020

The extra characters are being inserted by the web browser because the unescaped < character in <?php hits the HTML parser, which then tries "fix" the invalid HTML. You can test this by pasting your code sample into an empty .html file and opening it the web browser, the developer tools will show a parsed HTML tree like:

Screen Shot 2020-09-09 at 10 46 04

The easiest fix is to swap < out for its HTML entity &lt;. Alternatively, you can wrap the whole code sample in a <script type="text/template"> tag to tell the HTML parser to ignore it:

<pre><code class="php"><script type="text/template">
<?php

function myFunc(){
  echo 'test';
}

$name = 'myFunc';
$name();
</script></code></pre>

Multiple people have reported that this worked in prior releases. While the issues has always been there for code added via HTML, I think the change in 4.0 is that it somehow started affecting external Markdown as well. I'm leaving this issue open until that par has been debugged further, if anyone has any ideas as to what change caused this I'm all ears.

@FrancoSpeziali
Copy link

@hakimel Thanks, I figured it was something like that. I also tried swapping out the < > with HTML entities, but it's not idea for me. When I have some free time I would like to investigate further.

@bobbens
Copy link

bobbens commented Sep 9, 2020

It affects inline markdown as well. I didn't have an issue with 3.x and have it with 4.x. I've narrowed it down to the markdown plugin, but wasn't able to solve it.

Since I use a static website generator, my "solution" was to basically add a pre-processor that targets the ``` blocks and changes all the < to &lt; and > to &gt; and it works fine.

@hakimel
Copy link
Owner

hakimel commented Sep 9, 2020

I've fixed this in the dev branch—if you're using Markdown, please try the latest version from there and let me know if it works.

The issue was that as of 4.0 we started overriding the markdown parser's code renderer (https://github.com/hakimel/reveal.js/blob/dev/plugin/markdown/plugin.js#L429-L450) in order to support line numbers and line highlights. The built-in code renderer automatically escaped HTML entities in code, but our overridden version didn't. I've added HTML escapes to our own code renderer now too so it should continue to work just like it did pre 4.0.

@greg-witczak
Copy link
Author

Cool, thanks for fixing that! Unfortunately, I'm not able to check the fix, since I'm using reveal-md - so I'll let you know once it gets to regular release and reveal-md gets updated.

github-actions bot added a commit to vlaci/nix-doom-emacs that referenced this issue Sep 11, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
vlaci pushed a commit to vlaci/nix-doom-emacs that referenced this issue Sep 17, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
vlaci pushed a commit to vlaci/nix-doom-emacs that referenced this issue Sep 17, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
@517G
Copy link

517G commented Dec 6, 2020

README.md

R0bes pushed a commit to R0bes/Terraform-Presentation that referenced this issue Jun 7, 2021
harryleesan pushed a commit to harryleesan/reveal.js that referenced this issue Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants
@bobbens @hakimel @greg-witczak @FrancoSpeziali @topdeal2021 @517G and others