Fix fontURL issues #356
Fix fontURL issues #356
Conversation
lib/main.js Outdated
| | ||
| // fontURL to current MathJax version | ||
| if (!fontURL) | ||
| fontURL = 'https://cdnjs.cloudflare.com/ajax/libs/mathjax/' + MathJax.version + '/fonts/HTML-CSS'; |
There was a problem hiding this comment.
Probably should be indented, if not braced (Volker would vote for braces).
There was a problem hiding this comment.
Oh, that should have been on the same line (otherwise we overwrite a custom fontURL, no?)
There was a problem hiding this comment.
No, it is still part of the if statement, even without braces. The newline doesn't change the meaning. Since it is a long expression, having it on a different line is fine. Using braces would not be a bad idea (and will make Volker happy).
There was a problem hiding this comment.
will make @zorkow happy
Now how could I refuse that 😄
| css: true | ||
| }, function (result, data) { | ||
| t.ok(result.css.indexOf('https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.0/fonts/HTML-CSS') > -1, 'Default fontURL'); | ||
| t.ok(result.css.indexOf('https://cdnjs.cloudflare.com/ajax/libs/mathjax/' + mjVersion + '/fonts/HTML-CSS') > -1, 'Default fontURL'); |
There was a problem hiding this comment.
The other option, I suppose, is to do a regex search with mjVersion replaced by something like \d+\.\d+\.\d+. But I'm OK with the lookup as is.
There was a problem hiding this comment.
Ah, nice idea. I somewhat favor this one but I'm ok either way.
addresses review of #356
| @dvpc PTAL |
| LGTM |
Fixes #355