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

TeX annotation in MathML output mode should be XML escaped #935

Closed
physikerwelt opened this issue Oct 14, 2014 · 9 comments
Closed

TeX annotation in MathML output mode should be XML escaped #935

physikerwelt opened this issue Oct 14, 2014 · 9 comments
Assignees
Labels
Accepted Issue has been reproduced by MathJax team Fixed Test Available v2.5
Milestone

Comments

@physikerwelt
Copy link
Contributor

It would be nice if the TeX annotation generted in the MathML output would be XML escaped.
Otherwise, XML validation tools might cosinder the characters & < >... to be invalid.
In https://gerrit.wikimedia.org/r/#/c/166556/ provides a workaround for most cases, but if the TeX string contains "" we could have a problem with the woraround.
As far as I know this is a MathJax and not a MathJax node bug, but feel free to move the bug if I'm wrong.

@physikerwelt
Copy link
Contributor Author

I think it sould be fixed here.
https://github.com/physikerwelt/MathJax/blob/master/unpacked/extensions/toMathML.js#L164
However, I don't know how if there is already an XML escaping methoid build into mathjax that could be used.
@dpvc: Is there a method for XML escaping?

@pkra
Copy link
Contributor

pkra commented Oct 14, 2014

The reason is probably that we didn't expect MathJax-node, i.e., the only application would be "Show Source" in the menu. Here's what it's doing https://github.com/physikerwelt/MathJax/blob/master/unpacked/extensions/MathMenu.js#L760-L761

IIUC the best practice within XML is to use CDATA (e.g., http://jats.nlm.nih.gov/archiving/tag-library/1.1d1/n-skz0.html) but that wouldn't work in an HTML settings -- and I think we'd need both. So yes, I'd say that's reasonable but I'm not sure whether we need an XML / CDATA option.

@davidcarlisle
Copy link

On 14 October 2014 17:06, Peter Krautzberger notifications@github.com
wrote:

The reason is probably that we didn't expect MathJax-node, i.e., the only
application would be "Show Source" in the menu. Here's what it's doing
https://github.com/physikerwelt/MathJax/blob/master/unpacked/extensions/MathMenu.js#L760-L761

IIUC the best practice within XML is to use CDATA (e.g.,
http://jats.nlm.nih.gov/archiving/tag-library/1.1d1/n-skz0.html) but that
wouldn't work in an HTML settings -- and I think we'd need both. So yes,
I'd say that's reasonable but I'm not sure whether we need an XML / CDATA
option.

I think even in a pure xml context it's better to quote <>& rather than use
CDATA, apart from anything else it ensures ]]> is made safe

David

@physikerwelt
Copy link
Contributor Author

I have a fix that works for me. I'll create a pull request soon.
https://github.com/physikerwelt/MathJax/tree/mathoid

@pkra
Copy link
Contributor

pkra commented Oct 15, 2014

Thanks, @davidcarlisle and thanks @physikerwelt for working on a pull request.

@dpvc dpvc added this to the MathJax 2.5 milestone Nov 20, 2014
@dpvc dpvc added the Accepted Issue has been reproduced by MathJax team label Nov 20, 2014
dpvc added a commit that referenced this issue Nov 20, 2014
XML-escape TeX annotation (Resolves issue #935)
@dpvc dpvc added Merged Merged into develop branch Test Needed labels Nov 20, 2014
@dpvc dpvc closed this as completed Nov 20, 2014
@pkra
Copy link
Contributor

pkra commented Dec 4, 2014

Is a treeReftest sufficient? E.g., \(&amp;&lt;&gt;\) with

      <math xmlns="http://www.w3.org/1998/Math/MathML">
        <semantics>
          <merror>
            <mtext>&amp;&lt;&gt;</mtext>
          </merror>
          <annotation encoding="application/x-tex">&amp;&lt;&gt;</annotation>
        </semantics>
      </math>

@pkra
Copy link
Contributor

pkra commented Dec 4, 2014

Oh nevermind. I should remember my TeX...

@dpvc
Copy link
Member

dpvc commented Dec 4, 2014

Is a treeReftest sufficient?

No, you will need to use a script test, because the change is in toMathML(), not the NativeMML output. So you need to check the results of toMathML() to see that the annotation contains the escaped characters.

@pkra pkra self-assigned this Dec 4, 2014
pkra added a commit to mathjax/MathJax-test that referenced this issue Jan 15, 2015
dpvc added a commit to mathjax/MathJax-test that referenced this issue Jan 15, 2015
@dpvc
Copy link
Member

dpvc commented Jan 15, 2015

==> In Testsuite

UI/issue935.html

@dpvc dpvc added Fixed v2.5 and removed Merged Merged into develop branch labels Jan 30, 2015
physikerwelt added a commit to wikimedia/mediawiki-services-mathoid that referenced this issue Jul 31, 2015
* for some reason the TeX annotation was removed
  again by accident
* reintroduce this feature originally fixed in
  I79cf4e0ac40feae97da1dad8a172a6fc0d7987de
* the escaping issue was fixed in upstream
  mathjax/MathJax#935

Bug: T73673
Change-Id: I5b05e6d7c1947755c7715396468137fe6a0d56f7
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 Available v2.5
Projects
None yet
Development

No branches or pull requests

4 participants