-
Notifications
You must be signed in to change notification settings - Fork 16
fix: inline latex rendering #90
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
Conversation
Summary of ChangesHello @xihale, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where inline LaTeX mathematical expressions were not being rendered as intended. By updating the KaTeX auto-render script's configuration to include specific delimiters for both inline and block-level math, the change ensures that all LaTeX syntax is correctly recognized and displayed, significantly improving the presentation of mathematical content. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request fixes an issue with inline LaTeX rendering by configuring custom delimiters for KaTeX, correctly adding support for single-dollar-sign syntax. However, a high-severity DOM-based Cross-Site Scripting (XSS) vulnerability has been identified. This arises because the KaTeX renderer processes the entire document body, which contains unsanitized user-generated content, creating a risk of script execution if malicious content is submitted. Additionally, the current implementation embeds complex JavaScript logic within an onload attribute, which could be refactored for better readability and maintainability by extracting it into a separate deferred script block.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request fixes inline LaTeX rendering by correctly configuring KaTeX to use single dollar signs for inline math. The implementation uses a DOMContentLoaded event listener. My review suggests an alternative approach using the onload attribute on the script tag. This would make the implementation more robust against script loading failures and more consistent with how other scripts are loaded in the same file.
layouts/post.shtml
Outdated
| <script defer src="https://mirrors.sustech.edu.cn/cdnjs/ajax/libs/KaTeX/0.16.9/contrib/auto-render.min.js" integrity="sha512-iWiuBS5nt6r60fCz26Nd0Zqe0nbk1ZTIQbl3Kv7kYsX+yKMUFHzjaH2+AnM6vp2Xs+gNmaBAVWJjSmuPw76Efg==" crossorigin="anonymous"></script> | ||
| <script> | ||
| document.addEventListener("DOMContentLoaded", function() { | ||
| renderMathInElement(document.body, { | ||
| delimiters: [ | ||
| {left: '$$', right: '$$', display: true}, | ||
| {left: '$', right: '$', display: false}, | ||
| {left: '\\(', right: '\\)', display: false}, | ||
| {left: '\\[', right: '\\]', display: true} | ||
| ] | ||
| }); | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach using DOMContentLoaded works, but it can be made more robust and consistent with other scripts on the page. If the KaTeX script fails to load, this will cause a ReferenceError in the console.
A more robust pattern, which is also consistent with how mermaid.js is loaded in this file, is to use the onload attribute. This ensures the rendering function is only called after the script has successfully loaded, avoiding console errors on script load failure.
I suggest defining a function for rendering and calling it from the onload attribute. This also keeps the configuration nicely separated from the script tag.
<script>
function renderKaTeX() {
renderMathInElement(document.body, {
delimiters: [
{left: '$$', right: '$$', display: true},
{left: '$', right: '$', display: false},
{left: '\\(', right: '\\)', display: false},
{left: '\\[', right: '\\]', display: true}
]
});
}
</script>
<script defer src="https://mirrors.sustech.edu.cn/cdnjs/ajax/libs/KaTeX/0.16.9/contrib/auto-render.min.js" integrity="sha512-iWiuBS5nt6r60fCz26Nd0Zqe0nbk1ZTIQbl3Kv7kYsX+yKMUFHzjaH2+AnM6vp2Xs+gNmaBAVWJjSmuPw76Efg==" crossorigin="anonymous" onload="renderKaTeX()"></script>
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to fix inline LaTeX rendering by explicitly configuring KaTeX delimiters, including support for single-dollar signs ($...$). However, it introduces a potential DOM-based Cross-Site Scripting (XSS) vulnerability due to processing user-controlled content with the renderMathInElement function without proper sanitization, which could allow arbitrary script execution. Additionally, it's recommended to refactor the script loading and execution to use DOMContentLoaded for better maintainability and to avoid global namespace pollution.
* fix: inline latex rendering * refactor(katex): extract config to separate script
Close #87