search

mathjax / MathJax-node

MathJax for Node
Apache License 2.0
615 stars 97 forks source link

[main.js] add missing checks for htmlNode #289

Closed pkra closed 7 years ago

pkra commented 7 years ago

Fixes #288

dpvc commented 7 years ago

This looks OK, but I think that there may need to be one more change in order to handle requests for css when there is no request for html or htmlNode. You may want to move line 594 to the top of that function, right before line 566.

pkra commented 7 years ago

Ah! And I'm guessing https://github.com/mathjax/MathJax-node/blob/f1946b2aee3d3eab96fb5aa13e7e8f7f116fa9a2/lib/main.js#L687 needs to check for css as well so that typesetting happens with CHTML and the styles get generated, right?

dpvc commented 7 years ago

No, the styles are obtained just by loading the output jax, and that always happens. So you don't need to typeset using CHTML to get that. (I haven't tested that, but I'm pretty sure that's true.)

pkra commented 7 years ago

I haven't tested that, but I'm pretty sure that's true.

I've tested it and it fails for me, see #292. I think we should merge this and deal with the other issue separately.

dpvc commented 7 years ago

Since the fix for data.css is in the same line (687) as this change, I'd say doing as part of this PR would be reasonable.

pkra commented 7 years ago

In the interest of concise commits, can I go ahead and merge?

dpvc commented 7 years ago

OK, sure.