Added MathJax compatibility script by wilbowma · Pull Request #680 · KaTeX/KaTeX

wilbowma · 2017-04-20T18:14:03Z

This script helps a user easily switch from MathJax to KaTeX.

gagern

Interesting suggestion, thank you very much. Having this as a contributed piece of code is exactly the right thing to do, in my opinion.

gagern · 2017-04-20T21:46:29Z

contrib/mathjax-compat/mathjax-compat.js

+function replaceMathJaxDisplay(node) {
+    const katexinline = document.createElement("div");
+    katexinline.setAttribute("class", "equation");
+    katexinline.innerHTML = "\\displaystyle " + katex.renderToString(node.text);


I'd prefer katex.render(node.text, katexinline, {displayMode: true}), i.e. using the DOM renderer instead of innerHTML and using the displayMode option instead of a \displaystyle as part of the formula. Unless you have a good reason to do otherwise. displayMode improves vertical spacing around the equation, and render to DOM might in the future allow some tricks that are impossible when rendering to string.

gagern · 2017-04-20T21:47:50Z

contrib/mathjax-compat/mathjax-compat.js

+        if (s.type === "math/tex") {
+            replaceMathJaxInline(s);
+        }
+        if (s.type === "math/tex; display") {


The exact match here looks very restrictive, with respect to spacing or additional parameters. Do you have any reference how MathJax matches its types?

Personally I've used code like this before:

var scripts = document.body.getElementsByTagName("script"); scripts = Array.prototype.slice.call(scripts); scripts.forEach(function(script) { if (!/^text\/(x-)?([lk]a)?tex($|;)/.test(script.type)) return; var display = (/;mode=display($|;)/.test(script.type)); var elt = document.createElement(display ? "div" : "span"); try { katex.render(script.text, elt, {displayMode: display}); } catch (err) { console.error(err); elt.textContent = script.text; } script.parentNode.replaceChild(elt, script); });

This is not compatible, since it dies not allow a space at all, and it requires a mode= before the display. But it may contain some useful suggestions, e.g. supporting a wider set of possible types, or handling errors by printing the equation as plain text instead. Since text/tex is not a registered MIME type, using it without a leading x- is probably violating some standard or other.

You're right, there are definitely some bugs in this code. There at least ought to be a "mode=" before the display, and probably ought to compare case-insensitively. Unfortunately, I'm having trouble finding how MathJax does this. I'll keep looking.

Looks like MathJax does two checks. The first check removes / *;.*/ (i.e. whitespace and then a semicolon and everything after the semicolon) and then checks for "math/tex".

Then, the second check looks for an instance of /(;|\s|\n)mode\s*=\s*display(;|\s|\n|$)/ to decide if it should be in display mode. Those could probably be combined into a single check, I think it's just done in two parts because of how MathJax is structured.

gagern · 2017-04-20T21:54:02Z

contrib/mathjax-compat/Makefile

+
+integrity: $(BUILDDIR)/contrib/mathjax-compat.min.js
+	$(eval TMP := $(shell openssl dgst -sha384 -binary $< | openssl base64 -A))
+	sed -i s/integrity=\".*\"/integrity=\"sha384-$(subst /,\\/,$(TMP))\"/  README.md


The update-sri.js invocation in release.sh should take care of this. Please check it does, by writing a garbled hash then running release.sh --dry-run and verifying that the README has been modified to match the built file.

gagern · 2017-04-20T21:57:10Z

contrib/mathjax-compat/README.md

@@ -0,0 +1,15 @@
+# MathJax Compatibility Extension


I'm not sure about the name. On the one hand, most MathJax users will probably use delimiter-separated text like $…$ instead of scripts, while at the other hand it makes a lot of sense to use scripts even when you are originally writing for KaTeX. I do so myself. So the name should concentrate on TeX code contained in <script> elements, not on the fact that MathJax happens to support that same approach.

It might make sense to mention that MathJax supports the same approach, but I agree that it should be called something more related to what it actually does.

That's a good point. I don't actually use MathJax directly, but through a Markdown parser that generates output in using the custom script types that MathJax can understand. I figured that was the normal interface, but reading the docs I see I'm wrong.

wilbowma · 2017-04-20T22:48:51Z

@gagern, thanks a lot for the feedback! I'm pretty new to both MathJax and KaTeX, but wanted to try to contribute some of my personal scripts for wider use.

gagern · 2017-04-20T23:18:49Z

https://github.com/mathjax/MathJax/blob/2.7.0/unpacked/jax/input/TeX/jax.js#L2168 does the mode detection. I haven't found the mime type matching yet.

wilbowma

I'm not sure why this commit caused tests to start failing, and I don't understand the output from Travis. Any advice would be appreciated.

gagern · 2017-04-22T20:49:00Z

The Selenium run by Travis is having problems setting up a fixed browser window size for the screenshots. Reason unknown but surely unrelated to your changes. I triggered a rerun of the tests. If this happens more often we may have to investigate in more detail.

edemaine · 2017-04-22T21:14:33Z

FWIW, I've seen this now and then locally and in other pull requests (specifically #670). Maybe a time delay needs to be increased...

gagern · 2017-04-23T09:20:16Z

OK, Travis is happy now.

A bit of background on the screenshot sizes: with selenium we can configure the window size and then observe the size of the screenshot taken from the document area of that window. We don't want to hard-code the amount of space taken up by toolbars and so on. So setSize will iteratively adjust the window size until it gets a screenshot of the desired size. This worked even in the failing run here. Then it goes for the actual screenshots for test inputs, and there it verifies that the size of those screenshots still matches the one it obtained before. It's this sanity check that failed in the previous run here.

If we can reproduce this issue with some reasonable probability, it would be interesting to run this using one of the debug docker images (for Chrome resp. Firefox) to see whether we can work out what's going on with that window. Will see whether I can reproduce locally…

k4b7 · 2017-05-13T19:46:44Z

contrib/mathtex-script-type/README.md

+
+```html
+<script src="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/contrib/mathtex-script-type.min.js" integrity="sha384-o+v+EkJWQmZj7XwHBxehTGJKE18182WyyN2glZMTPw9g5XxjN1uwrquNuMX/NJiF"></script>
+```


Could you expand on this script a bit more? I assume that I need to load KaTeX somewhere too.

Maybe something along these lines:

<html> <head> <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/katex.min.css" integrity="sha384-wITovz90syo1dJWVh32uuETPVEtGigN07tkttEqPv+uR2SE/mbQcG7ATL28aI9H0" crossorigin="anonymous"> <script src="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/katex.min.js" integrity="sha384-/y1Nn9+QQAipbNQWU65krzJralCnuOasHncUFXGkdwntGeSvQicrYkiUBwsgUqc1" crossorigin="anonymous"></script> </head> <body> <script type="math/tex">x+\sqrt{1-x^2}</script> <script src="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/contrib/mathtex-script-type.min.js" integrity="sha384-o+v+EkJWQmZj7XwHBxehTGJKE18182WyyN2glZMTPw9g5XxjN1uwrquNuMX/NJiF"></script> </body> </html>

@wilbowma Could you take a pass at this?

@edemaine, sorry about that. Got busy with other things and forgot about this. I'll take a look today.

k4b7 · 2017-05-13T19:47:38Z

contrib/mathtex-script-type/README.md

+This include should be loaded after all `script` blocks you want to render.
+
+```html
+<script src="https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.7.1/contrib/mathtex-script-type.min.js" integrity="sha384-o+v+EkJWQmZj7XwHBxehTGJKE18182WyyN2glZMTPw9g5XxjN1uwrquNuMX/NJiF"></script>


Will this actually work at the moment? Wouldn't we have to publish a 0.7.2 version of the KaTeX in order for mathex-script-type.min.js to appear in the published package?

I wonder if we should just encourage people to copy/paste the script manually before we do a publish and then update this to point to the published script afterwards.

Looking at the script though, we'd probably want to run make build/contrib and use the built version of the script.

Are you suggesting that the Makefile automatically tweak the README to include the latest version number? This seems like it'd be cool, though I don't quite know how it'd work...

Perhaps a simpler solution would be to write <script src=".../KaTeX/contrib/mathtex-script-type.min.js"></script> and let users fill in the right path, either local to their webserver (as many are probably using it) or on CDN? We could also give a template for CDN including, but with ... or something else in place of the actual version number.

I worded that poorly. I think <script src=".../KaTeX/contrib/mathtex-script-type.min.js"></script> makes sense. The README could be updated in a separate diff to point to the CDN version, but as this stands right now this code won't work.

I used this URL based on user-contrib/auto-render/README.md. You're right, of course, that this path won't work just now, but I assumed that this script would be handled similarly to auto-render.

The trouble is that once we merge your pull request, the README immediately becomes publicly visible. So we should probably use ... for now, and update after a release (or change them all to ...).

Also, don't the build scripts automatically update the integreity hashes in the contrib README? (see above comment on earlier version of my Makefile.) Surely they can be modified to update the version in the CDN string.

Ah of course. Okay.

Oops, indeed, update-sri.js already updates the version number. It looks like it will replace any URL with any version number, including ..., so we could still use that until a release happens. Then the release will fix it to a working version number.

edemaine · 2017-06-11T17:23:10Z

Sorry, now that I understand what the release script does, it makes sense to keep CDN URLs, but just change the version number part to ... where it doesn't yet exist (your new file).

This reverts commit 857f8de.

edemaine · 2017-06-11T18:02:10Z

Thanks @wilbowma! This looks good to me. @kevinbarabash did you want to confirm the revisions address your review?

k4b7 · 2017-08-23T23:40:40Z

@wilbowma sorry about the delay. Thank you for the PR. It looks like release.sh will take care of updating the integrity SHAs. I'd like to the merge right before I publish but I probably won't get to the publish until tomorrow evening.

k4b7 · 2017-08-27T22:12:50Z

I forgot to include this in v0.8.3 release so I'm going to merge this now so that I goes out in the v0.8.4 release for sure. I'll post a separate PR to update the integrity SHAs.

k4b7 · 2017-08-28T03:13:59Z

I just realized that posting a separate PR to update the integrity SHAs doesn't make sense b/c the publish has to happen for the new files to be uploaded to CDNs.

Added MathJax compatibility script

9aee9eb

gagern reviewed Apr 20, 2017

View reviewed changes

wilbowma added 5 commits April 20, 2017 23:47

Removed make integrity, which release.sh handles

c55e35c

Renamed scripts

27d49df

Use katex.render (and options) instead of manually string munging

5e01d12

Rewrote code based on feedback

a533149

Renamed file correctly

4c02073

wilbowma mentioned this pull request Apr 22, 2017

Added KaTeX widget, and MathJax compatibility layer greghendershott/frog#190

Open

wilbowma commented Apr 22, 2017

View reviewed changes

k4b7 added the GH Review: review-needed label May 13, 2017

k4b7 requested changes May 13, 2017

View reviewed changes

k4b7 added GH Review: needs-revision and removed GH Review: review-needed labels May 13, 2017

wilbowma added 2 commits June 11, 2017 12:58

Extended README with an example of usage

9e62006

Removed CDN paths in place of an abstract path

857f8de

wilbowma added 3 commits June 11, 2017 13:53

Revert "Removed CDN paths in place of an abstract path"

ec63bf2

This reverts commit 857f8de.

Replace versions with ..., with note to avoid confusing users

880981e

Revert some ... replacements to files that do exist.

04453ce

Merge branch 'master' into master

512950b

k4b7 approved these changes Aug 23, 2017

View reviewed changes

Merge branch 'master' into master

4606cc2

k4b7 removed the GH Review: needs-revision label Aug 23, 2017

Merge branch 'master' into master

fb85ef5

k4b7 merged commit 2fbb2b0 into KaTeX:master Aug 27, 2017

Uh oh!

Conversation

wilbowma commented Apr 20, 2017

Uh oh!

gagern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilbowma commented Apr 20, 2017

Uh oh!

gagern commented Apr 20, 2017

Uh oh!

wilbowma left a comment

Choose a reason for hiding this comment

Uh oh!

gagern commented Apr 22, 2017

Uh oh!

edemaine commented Apr 22, 2017

Uh oh!

gagern commented Apr 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edemaine commented Jun 11, 2017

Uh oh!

edemaine commented Jun 11, 2017

Uh oh!

k4b7 commented Aug 23, 2017

Uh oh!

k4b7 commented Aug 27, 2017

Uh oh!

k4b7 commented Aug 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants