-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat!: upgrade to mathjax v3 #33555
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
base: master
Are you sure you want to change the base?
feat!: upgrade to mathjax v3 #33555
Conversation
|
Thanks for the pull request, @navinkarkera! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
3f5125c to
6d29ae7
Compare
|
@navinkarkera Great work on this! |
@CefBoud Although I don't know if it is in the scope, I can update it as it seems to be used in only once place. |
6d29ae7 to
70f67b9
Compare
CefBoud
left a comment
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.
👍
- I tested this: in Text, Numerical Input, Math Expression and Problem components. Also in course Discussions. Inline and display expressions.
- I read through the code
- I checked for accessibility issues : Mathjax built-in accessibility functioning properly. Tested with Chrome Screen Reader extension.
-
Includes documentation -
I made sure any change in configuration variables is reflected in the corresponding client's.configuration-securerepository
70f67b9 to
5315c9c
Compare
fix: use asciimath test: fix mathjax related tests fix: lint issues fix: mathjax_delay_renderer test: fix mathjax related tests test: fix formula_equation_preview tests
5315c9c to
c7ed9e1
Compare
|
Please see discussion re upgrade paths for MathML rendering not being simple in this earlier PR: #32418 |
|
@wittjeff Thanks for pointing me to the PR. About implementing course level advance option to support both v2 and v3, I'll come back with an update/answer soon. |
| $script( | ||
| 'https://cdn.jsdelivr.net/npm/[email protected]/MathJax.js' | ||
| + '?config=TeX-MML-AM_SVG&delayStartupUntil=configured', | ||
| 'https://cdn.jsdelivr.net/npm/[email protected]/es5/tex-mml-svg.js', |
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.
There is a v3.2.2 available. https://github.com/mathjax/MathJax/releases/tag/3.2.2
|
|
||
| // externally hosted files | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@2.7.5/MathJax.js?config=TeX-MML-AM_SVG&delayStartupUntil=configured', // eslint-disable-line max-len | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@3.2.1/es5/tex-mml-svg.js?noext', |
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.
3.2.2 available
| 'URI': 'xmodule_js/common_static/js/vendor/URI.min', | ||
| 'mock-ajax': 'xmodule_js/common_static/js/vendor/mock-ajax', | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@2.7.5/MathJax.js?config=TeX-MML-AM_SVG&delayStartupUntil=configured', // eslint-disable-line max-len | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@3.2.1/es5/tex-mml-svg.js?noext', |
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.
3.2.2 available
| 'domReady': 'xmodule_js/common_static/js/vendor/domReady', | ||
| 'URI': 'xmodule_js/common_static/js/vendor/URI.min', | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@2.7.5/MathJax.js?config=TeX-MML-AM_SVG&delayStartupUntil=configured', // eslint-disable-line max-len | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@3.2.1/es5/tex-mml-svg.js?noext', |
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.
3.2.2 available
| @import 'build-v1'; // shared app style assets/rendering | ||
|
|
||
|
|
||
| .MathJax>svg { |
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.
I thought v3 eliminated SVG rendering. No? SVG rendering is problematic when used as items in a select element.
| ], | ||
| autoload: { | ||
| color: [], | ||
| colorv2: ['color'] |
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.
Do we need to consider accessibility (particularly contrast) of a MathJax color palette?
| } | ||
| }; | ||
| vendorScript.src = 'https://cdn.jsdelivr.net/npm/[email protected]/MathJax.js?config=TeX-MML-AM_HTMLorMML'; | ||
| vendorScript.src = 'https://cdn.jsdelivr.net/npm/[email protected]/es5/tex-mml-svg.js'; |
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.
3.2.2 available
| It can't be run through static.url because MathJax uses crazy url introspection to do lazy loading of | ||
| MathJax extension libraries --> | ||
| <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/mathjax@2.7.5/MathJax.js?config=TeX-MML-AM_SVG"></script> | ||
| <script type="text/javascript" id="MathJax-script" src="https://cdn.jsdelivr.net/npm/mathjax@3.2.1/es5/tex-mml-svg.js"></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.
3.2.2 available
| 'jasmine-imagediff': 'xmodule_js/common_static/js/vendor/jasmine-imagediff', | ||
| 'domReady': 'xmodule_js/common_static/js/vendor/domReady', | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@2.7.5/MathJax.js?config=TeX-MML-AM_HTMLorMML&delayStartupUntil=configured', // eslint-disable-line max-len | ||
| mathjax: 'https://cdn.jsdelivr.net/npm/mathjax@3.2.1/es5/tex-mml-svg.js?noext', |
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.
3.2.2 available
| } | ||
| } | ||
|
|
||
| .MathJax>svg { |
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.
Use of SVG??
|
Related discussion re Canvas use of MathJax: https://community.canvaslms.com/t5/Canvas-Question-Forum/Canvas-s-MathJax-Needs-An-Update/m-p/583189 |
|
I submitted a PR because I'm having some issues with MathJax on the edX platform.Let me know if there is anything I can do here |
|
Hi @navinkarkera, I tried testing this recently with a more current version of edx-platform and could not get it to work. Is it still working on your environment? |
|
@rayzhou-bit Thanks for testing it. I am currently occupied with another project and we haven't received any confirmation from the original stakeholders yet, but I hope to get back to this soon. |
OmarIthawi
left a comment
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.
Thanks @navinkarkera for your contribution. This is really useful and the update is much needed to improve performance.
I recommend using HTML CSS instead of SVG whenever possible. SVG is challenging for MathJax plugin makers like myself when it comes to Arabic and RTL support.
Additionally, Open edX needs to support mobile and to stay accessible which I think MathJax SVG isn't very well suited for:
|
Hi @navinkarkera! Are you planning to pursue this PR? |
|
@mphilbrick211 Yes, once mathjax 4 is released, we plan to upgrade since it is already in beta. For more info, please see this discussion |
|
Hi @navinkarkera! Just checking in on this :) |
|
@mphilbrick211 Sorry, I don't have any update here. |
|
Version 4.0.0 is now available. |
|
@navinkarkera I'm gonna move this to draft until you're ready to with the MathJax 4.0 update. Once it's ready for review let me know and I can help get it reviewed and landed. |
Description
Upgrades mathjax from version 2 to 3.2.1. MathJax v3 is a complete rewrite of MathJax with changes in its internal structure so it is not a drop in replacement to v2.
This PR implements the changes required to make use of v3 and still have the same functionality as before and hopefully without breaking anything.
Useful information to include:
Supporting information
Private-ref: BB-7992Testing instructions
make {lms,cms,frontend-app-learning}-up.make {lms,cms}-static.make {lms,cms}-restartto make sure new static files are served.mathjaxto make sure that mathjax v3 is being loaded.\[\mathbf{x}^{G} = \frac{1} {M}\sum^{3}_{i=1}m_i\mathbf{x}^{G}_i\]formulae in the text.R_1*R_2/R_3,A*x^2 + sqrt(y). Make sure preview works as expected.Mathematical Expressionsin second section and play with the input field. In this field you can also enter tex as it is directly converted by MathJax while numerical input problems use a python library calledcalcto parse the text first and they cannot parse tex.Chemical Equations, play with the input field.View Live Versionbutton and test above blocks again in LMS.\, example:\`A*x^2 + sqrt(y)\`\[and\(with additional\for tex commands, example:\\[A \cdot x^2 + \sqrt{y}\\],\\(A \cdot x^2 + \sqrt{y}\\)Please let me know if we are using mathjax in some other components which we need to test.
Deadline
None
Other information
None