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 scripts and href in HTML-CSS output #364

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

Comments

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

commented Dec 19, 2012

The following code generates a Math Processing Error

 <math>
    <mover>
      <mtext href="http://www.mathjax.org">MathJax</mtext>
      <mi>x</mi>
    </mover>
  </math>

The important parts seem to be the script element (you can try with munderover, msub etc) and the href (you can change the mtext by another element). The error is the following:

boxes[i].bbox is undefined
file http://cdn.mathjax.org/mathjax/latest/unpacked/jax/output/HTML-CSS/jax.js
line 2450

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Dec 24, 2012

I proposed a fix on my issue364 branch:

https://github.com/fred-wang/MathJax/compare/master...issue364

When there is a href attribute, HTMLcreateSpan creates a new "a" element before creating the span. At the end it sets the bbox on the parent "a" too but still returns the span child. I made it return the "a" element intead and that seems to make the test case work.

@ghost ghost assigned fred-wang Dec 24, 2012

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2013

Crashtests/issue364.html

=> In testsuite

@dpvc

This comment has been minimized.

Copy link
Member

commented Feb 12, 2013

I think the solution is actually in a different part of the code. The HTMLcreateSpan() has to return the inner span.

In this case is that this.boxes[i] is the result of HTMLCSS.createBox(), and its contents' positions are set via HTMLCSS.placeBox(); it is the latter that is supposed to set the bbox for the surrounding box. When a span is positioned in the box via placeBox, it looks for the span's parentNode and uses its bbox as the one for the surrounding box. When the span has an href, however, the parent node is the a not the span for the box.

So I suggest that placeBox is the right place to fix this error, by adding something like

if (parent.nodeName === "a") {parent = parent.parentNode}

or perhaps

while (parent && !parent.isBox) {parent = parent.parentNode}

so that you "bubble up" to the actual result of the createBox() call.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2013

I don't think that it will work, at least not on the testcase I give. Using the debugger, I see that the items of boxes already have isBox = true before the call to HTMLCSS.MeasureSpans ; after this call only the one without href has new HH and bbox properties ; then the boxes[i] is accessed and raise the error for the one with href ; placeBox is called later in that function.

@dpvc

This comment has been minimized.

Copy link
Member

commented Feb 13, 2013

OK, I see. Then perhaps the parent on line 785 (in MeasureSpans) also needs to be adjusted to take account of the surrounding <a>. Maybe it would be good to add a method to MML.mbase to do the parent node lookup so that it handles these things.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2013

It seems that modifying Measured to get the innermost isBox ancestor (at the two places where span.parentNode is called) fixes the issue on the testcase too, although I'm not sure to understand why and whether it is correct (if I modify placeBox too, I get an error). I'm probably not the best person to fix that issue, as I don't really know the HTML output code. @dpvc would you mind taking care of that issue?

=> unassigning myself

@ghost ghost assigned dpvc Apr 18, 2013

dpvc pushed a commit to dpvc/MathJax that referenced this issue Apr 28, 2013

@dpvc

This comment has been minimized.

Copy link
Member

commented Apr 28, 2013

The issue364 branch of my fork of MathJax includes a fix (it skips over the parent <a> element).

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2013

=> Ready for release

dpvc pushed a commit to dpvc/MathJax that referenced this issue Apr 29, 2013

@dpvc

This comment has been minimized.

Copy link
Member

commented Apr 29, 2013

=> Merged

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.