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

Replace non-standard MJX-arrow class by an menclose notation #481

Closed
fred-wang opened this issue May 23, 2013 · 10 comments

Comments

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

commented May 23, 2013

See

https://groups.google.com/forum/#!topic/mathjax-dev/HM_p8EgH48Y
https://groups.google.com/forum/#!topic/mathjax-dev/A0Y_WYrWIcQ

\cancelto generates

<menclose notation="updiagonalstrike" class="MJX-arrow">

Unfortunately, this won't with other MathML rendering engines. Since the menclose notations are open-ended, I suggest to add a notation "updiagonalarrow", report that to the MathML WG and other implementers.

MathJax could then generate

<menclose notation="updiagonalstrike updiagonalarrow">

so that rendering engines that do not know updiagonalarrow could still display the updiagonalstrike. Other rendering engines that have updiagonalarrow support could implement it in a way that the updiagonalarrow notation "overrides" the updiagonalstrike notation that is the previous markup would be equivalent to just

<menclose notation="updiagonalarrow">

@ghost ghost assigned fred-wang May 23, 2013

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2013

Regarding Davide's remark about how multiple values are handled:

In Gecko, when notation is "circle circle box left actuarial" the attribute is parsed into a bit mask that will lead to something equivalent to "circle left top bottom right" that is the two "circle" are merged and "box left actuarial" becomes "left top bottom right". Then the drawing is done from this canonical form: just draw an ellipse and left/top/bottom/right borders.

As I see in the HTML (and SVG) output, MathJax will split the string "circle circle box left actuarial" and add a drawing for each individual component: draw an ellipse for the first "circle", draw another ellipse for the second "circle", set border style to solid for the "box", set border-left to solid for "left" and set border-top/right to solid for "actuarial".

That's why I found easy to merge "updiagonalstrike updiagonalarrow" in Gecko but Davide expressed concerns about how it could be done by MathJax. Probably MathJax should reduce to a canonical form instead, before drawing the notations. That would make fixing this issue easier.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2013

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2013

The issue481 branch fixes this issue. For backward compatibility, the MJX-arrow class is still recognized by MathJax.

=> Ready For Review

@dpvc

This comment has been minimized.

Copy link
Member

commented May 30, 2013

Your fix looks good over all. I do have a couple of comments:

  • In cancel.js, I thought your solution was to use both UPDIAGONALSTRIKE and UPDIAGONALARROW so that browsers that don't implement UPDIAGONALARROW will still show the strike. And in the MathML jax, you replace the strike with the arrow, but I think you should add the arrow (if it isn't there), no?
  • In menclose.js (both of them), I think it would be better to do the strike and arrow as separate cases rather than a combined one. There is very little overlap between the two. They were done together in the original because the arrow was implemented through UPDIAGONALSTRIKE with a special marker internally. Since they are now two separate notations, there is no need to keep them together.
  • For the for (var n in notation), it is recommended that you always start with if (notation.hasOwnProperty(n) { so that you don't iterate over things like the object's methods and other properties that you aren't expecting to see. In your case, since there is a switch statement, and those properties won't match any of the cases, it doesn't hurt, other than that it slows it down a bit to search through all the cases when n is something like toString or one of the other properties for the object. But it is good to be in the habit of adding the check automatically so you don't forget it when it does matter.
  • Another technical detail, it is best to use === rather than == when you can (e.g., for string or numeric checking). The difference is that == does type coercion, while === doesn't, so when you know the types will be the same, it is more efficient to use ===. It is a good habit to use this automatically for string and number checking at least.

In the Firefox bug report, I see that they suggest that if both notations are used, then rendering both would not be a problem, as the one would lie on top of the other. There are two issues with that: 1) if anti-aliasing is done on the lines, that could cause the anti-aliasing to be twice as dark as it should be, so the line will be extra dark; but more important, 2) depending on how the arrow head is drawn, the strike line may cause the point of the arrow to be too wide (the arrow head will go to a point, while the line will not, so if they end at the same location, the line will be wider than the point, and will mess up the look of the arrow head). MathJax works hard to avoid that situation.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2013

OK, I've updated the branch. Indeed, I wrote everywhere that the notation should be "updiagonalstrike updiagonalarrow" in order to work with other MathML engines, but forgot to do that when writing the code :-( Note that if the MathML input receives notation="updiagonalarrow" it won't convert it to notation="updiagonalstrike updiagonalarrow" as I expect that people will continue to specify the two notations until updiagonalarrow becomes more widely implemented.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2013

BTW, "updiagonalarrow" won't work in a RTL context. I've never seen the cancelto notation in France so I have no ideas how widespread this notation is.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2013

LaTeXToMathML/cancel-1.html
MathMLToDisplay/Presentation/GeneralLayout/menclose/menclose-1p.html
MathMLToDisplay/Presentation/GeneralLayout/menclose/menclose-2p.html

=> In testsuite

@dpvc

This comment has been minimized.

Copy link
Member

commented Aug 1, 2013

OK, these changes look good. The only change I would suggest now is that the to property in the HTML-CSS menclose.js#L152 could be set initially to the value in L154 rather than changing it afterward. That had been done when the arrow and line were combined, but there is no need to set and then reset it here.

Other than that it is ready to go.

@fred-wang

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2013

For the record, here is the W3C MathML thread:
http://lists.w3.org/Archives/Public/www-math/2013Jun/0003.html
No complaints so far...

dpvc pushed a commit that referenced this issue Aug 2, 2013

@dpvc dpvc closed this Aug 2, 2013

@fred-wang

This comment has been minimized.

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.