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

mhchem TeX extension breaks internally when leaving out closing bracket #2835

Closed
wb-joona opened this issue Feb 14, 2022 · 5 comments
Closed
Labels
Accepted Issue has been reproduced by MathJax team Fixed Test Needed v3 v3.2
Milestone

Comments

@wb-joona
Copy link

wb-joona commented Feb 14, 2022

Issue Summary

TeX mhchem input extension produces an error from the internals when not closing the \ce command.

Steps to Reproduce:

Live minimal reproducible example here: https://codesandbox.io/s/nice-dhawan-yu49p?file=/index.html

  1. Load latest tex-chtml.js
  2. Render the equation \ce{H2O
  3. Notice uncaught error:
TypeError: t.slice is not a function
    at u.Machine (mhchem.js:1:410)
    at e.parse (tex-chtml.js:1:245691)
    at t.parse (tex-chtml.js:1:212776)
    at t.parse (tex-chtml.js:1:256801)
    at t.controlSequence (tex-chtml.js:1:220763)
    at e.t.parse (tex-chtml.js:1:243417)
    at t.parse (tex-chtml.js:1:212776)
    at t.parse (tex-chtml.js:1:256801)
    at t.Parse (tex-chtml.js:1:257409)
    at new t (tex-chtml.js:1:256142)
    at e.compile (tex-chtml.js:1:197284)
    at e.t.compile (tex-chtml.js:1:44978)
    at Object.renderMath (tex-chtml.js:1:35007)
    at e.renderConvert (tex-chtml.js:1:35820)
    at e.t.convert (tex-chtml.js:1:44865)
    at e.t.convert (tex-chtml.js:1:38921)
    at tex-chtml.js:1:22644
    at e (tex-chtml.js:1:717213)
    at new Promise (<anonymous>)
    at Object.e.handleRetriesFor (tex-chtml.js:1:717178)
    at Object.e.MathJax.<computed> [as tex2chtmlPromise] (tex-chtml.js:1:22597)
    at convert (yu49p.csb.app/:57:17)
    at HTMLInputElement.onclick (yu49p.csb.app/:133:11)

I would expect that with noerrors loaded, I would see some output at least. This error appears to break the rendering though.

The error appears to be originating here:
https://github.com/mathjax/MathJax-src/blob/2dd53ce6c8af3c9cceba0baf014ea9b065130774/ts/input/tex/mhchem/MhchemConfiguration.ts#L51-L53

The catch appears to assume that the error is an array, but the actual error emitted here is the following object:

{
  id: 'MissingCloseBrace',
  message: 'Missing close brace'
}

and thus there is no .slice().

Technical details:

  • MathJax Version: 3.2.0
  • Client OS: Windows 10 Pro
  • Browser: Chrome 98.0.4758.82

Supporting information:

  • Please supply a link to a (live) minimal example page, when possible.
  • If your issue is with the display of the mathematics produced by MathJax, include a screen snapshot that illustrates the problem, when possible.
  • Check your browser console window for any error messages, and include them here.
  • Include the MathJax configuration you are using, and the script tag that loads MathJax itself.
<script src="https://polyfill.io/v3/polyfill.min.js?features=es6"></script>
<script>
    window.MathJax = {
        loader: { load: ["[tex]/noerrors", "[tex]/mhchem"] },
        tex: { packages: { "[+]": ["noerrors", "mhchem"] } },
    };
</script>
<script
    id="MathJax-script"
    async
    src="https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-chtml.js"
></script>
@pkra
Copy link
Contributor

pkra commented Feb 14, 2022

https://github.com/mhchem/MathJax-mhchem/ is a better place for mhchem issues. mhchem/MathJax-mhchem#20 looks potentially similar.

@wb-joona
Copy link
Author

Thanks! Looks like a match indeed.

@wb-joona
Copy link
Author

On another thought, I'm not sure if the issue is with mhchem, but with the integration of the extension in the MathJax code. The issue is that the MhchemConfiguration file's catch assumes that the error is an array instead of an object, which is on MathJax' side - not mhchem's side.

@mhchem
Copy link

mhchem commented Feb 14, 2022

@wb-joona, thanks a lot for this bugreport, in particular your live minimal reproducible example. That was very helpful.

The issue is in MhchemConfiguration.ts

  try {
    let arg = parser.GetArgument(name);
    let tex = mhchemParser.toTex(arg, machine);
    parser.string = tex + parser.string.substr(parser.i);
    parser.i = 0;
  } catch (err) {
    throw new TexError(err[0], err[1], err.slice(2));
  }

Link to Source

The issue is that the first line (parser.GetArgument()) throws errors as TexError ({id: ..., message: ...}), while the mhchemParser in the 2nd line throws errors as an array of two strings (['id', 'message']).

As the TeXErrors are catched further above, anyway, it should be enough to "try" mhchemParser, shouldn't it? @pkra, @zorkow

  let arg = parser.GetArgument(name);
  try {
    let tex = mhchemParser.toTex(arg, machine);
  } catch (err) {
    throw new TexError(err[0], err[1]);
  }
  parser.string = tex + parser.string.substr(parser.i);
  parser.i = 0;

@dpvc
Copy link
Member

dpvc commented Feb 19, 2022

I have made a PR to address the issue based on @mhchem 's suggestion, which was right on target.

@dpvc dpvc added Accepted Issue has been reproduced by MathJax team Ready for Review Test Needed v3 labels Feb 19, 2022
@dpvc dpvc added this to the 3.2.1 milestone Feb 19, 2022
dpvc added a commit to mathjax/MathJax-src that referenced this issue Feb 22, 2022
Fix problem where errors during mhchem argument collection are not properly handled.  (mathjax/MathJax#2835)
@dpvc dpvc added Merged Merged into develop branch and removed Ready for Review labels Feb 22, 2022
@dpvc dpvc added Fixed v3.2 and removed Merged Merged into develop branch labels Jun 1, 2022
@dpvc dpvc closed this as completed Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Issue has been reproduced by MathJax team Fixed Test Needed v3 v3.2
Projects
None yet
Development

No branches or pull requests

4 participants