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

[Math Processing Error] with out-of-range maction@selection #365

Closed
fred-wang opened this issue Dec 19, 2012 · 17 comments

Comments

Projects
None yet
2 participants
@fred-wang
Copy link
Contributor

commented Dec 19, 2012

Things like

 <math>
    <maction actiontype="toggle" selection="0">
<mi>x</mi>
    </maction>
  </math>

or

  <math>
    <maction actiontype="toggle" selection="2">
    <mi>x</mi>
    </maction>
  </math>

generate a [Math Processing Error]. It should generate a message instead.

Error: "length is null"
file "http://cdn.mathjax.org/mathjax/latest/unpacked/jax/output/HTML-CSS/jax.js"
line 826

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Dec 20, 2012

Reading jax/element/mml/jax.js, it seems that MML.maction has a selected() member which will return MML.NULL when the selection is out of range. This causes an error later in both the HTML-CSS and SVG output Jax.

@dpvc

This comment has been minimized.

Copy link
Member

commented Dec 21, 2012

I guess the output jax should check for the error condition and either produce no output, or produce an <merror> indicating the problem.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2013

Crashtests/issue365.html

=> In testsuite

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Jan 14, 2013

This can be reproduced with LaTeX by creating an empty <maction>:

\require{action}\toggle\endtoggle

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

I added fix for this on my issue365 branch, that just does not displayed the frame when the selection is out of range. Actually that seemed to be the intended action but the test was verifying whether selection was null rather than whether is it MML.NULL.

@dpvc

This comment has been minimized.

Copy link
Member

commented Feb 8, 2013

OK, that looks good. I think MML.NULL was so that selected and other methods could always return an object,so that the result could be used like an MML object without getting the "x is undefined" error. I guess that didn't completely work.

I don't think I made much use of MML.NULL, and there are probably other places where it could be handy.

@dpvc

This comment has been minimized.

Copy link
Member

commented Feb 8, 2013

I wonder if \toggle\endtoggle should generate an error in the TeX input jax?

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

Yes, I think MML.NULL is used in the element jax to implement things like GetCoreMo for the maction.
I just run the fuzzer again and unfortunately that is not fixed completely. For example

<msub>
      <maction></maction>
      <mi>x</mi>
 </msub>

generate a (different) error. I guess this is because MML.NULL behaves differently in other places.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

That said, I noticed that the same error happens if the number of children of msub is incorrect, so I guess the appropriate fix is to better handle the invalid cases (for the moment, I've only reported processing errors with valid MathML trees).

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

(Actually, it was with

 <msubsup><maction></maction></msubsup>

so the number of argument is incorrect but the error seems rather due to the same reason: MML.NULL is used as the base)

@dpvc

This comment has been minimized.

Copy link
Member

commented Feb 8, 2013

Right, there is really no error checking for bad MathML structures (like wrong number of children, bad attribute values, etc.). Partly this was because such checks will make the code larger and slower to execute.

I think the right place for these checks is in the MathML input jax, and the TeX input jax (in the action and enclose extensions).

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

But this won't prevent errors if e.g. people write equation editors that directly work on the element Jax.

The new error is "bbox is null", unpacked/jax/output/HTML-CSS/jax.js, line 770.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2013

So I guess we must still insert something when the selected maction MML.NULL (either an empty frame or an merror). Or perhaps better not using MML.NULL at all but directly returns a MML.merror. Any way to do that?

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2013

OK, I updated my branch to better construct empty frame when he selected maction is MML.NULL. This time the changes pass fuzz testing.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2013

883d7a9

I've updated the branch to use this.HTMLzeroBBox and modified other places too where an explicit object was used (that may be why I didn't use HTMLzeroBBox).

@dpvc

This comment has been minimized.

Copy link
Member

commented Mar 21, 2013

Thanks. the HTMLzeroBBox routine came fairly late in the development and I guess I didn't go back and remove the other constant zero boxes.

FYI, if you refer to issue #365 in your commit message, it will be linked here automatically. I've been trying to do that (though I have an annoying habit of mistyping the number). I'm trying to do that for merges as well, so that it is easy to see the commit record here.

dpvc pushed a commit to dpvc/MathJax that referenced this issue Mar 21, 2013

@dpvc

This comment has been minimized.

Copy link
Member

commented Mar 21, 2013

Merged into develop.

@dpvc dpvc closed this May 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.