Improved MathJax, bug fixes #2517

Merged
merged 4 commits into from Nov 2, 2012

Conversation

Projects
None yet
4 participants
Contributor

ahmadia commented Oct 25, 2012

This pull request addresses the javascript errors resulting in a blank notebook pointed out in #2349

It also disables equation numbering/references (but see this comment for how to turn it back on), fixes broken offline access and re-typesets individual Notebook Cells instead of the entire document on edit.

If you are testing this out, please remember to clear your cache. Performance is addressed in this pull request, as well as the error resulting in a single cell notebook with the Markdown placeholder text "Type Markdown and LaTeX: α2"

ketch commented Oct 26, 2012

In my tests, this fixes the main problem I was having as a result of #2349. Notebooks always load correctly. The MathJax rendering speed seems to be greatly improved as well, though still a bit slower than in 0.13.

Owner

ellisonbg commented Oct 26, 2012

This branch is completely broken for me. I see the following:

Uncaught TypeError: Object [object Object] has no method 'typeset' textcell.js:403

Contributor

ahmadia commented Oct 26, 2012

This is very frustrating. I have time to work on this tomorrow, but there
are a few things to consider:

  • how important are equation line numbers and references? (which are
    introducing the complexity that both slows performance and I believe is
    breaking Brian's code)
  • should we revert the original commit from master until this is sorted out?
  • please give browser name, version, and operating system when reporting
    failures (and a stack trace if you know how to generate one)

I tested this against Safari and Chrome, the error Brian is seeing doesn't
make sense to me, but I'm going to check this whole approach out with the
MathJax developers.

A

Owner

ellisonbg commented Oct 26, 2012

On Fri, Oct 26, 2012 at 12:40 PM, ahmadia notifications@github.com wrote:

This is very frustrating. I have time to work on this tomorrow, but there
are a few things to consider:

Yes, very frustrating...thanks for looking at this.

  • how important are equation line numbers and references? (which are
    introducing the complexity that both slows performance and I believe is
    breaking Brian's code)

It would be nice to have it, but only if we can resolve these problems
without too much difficulty.

  • should we revert the original commit from master until this is sorted
    out?

Is it possible to create a PR that removes the references and line numbers
but keeps the general approach to MathJax+Markdown processing?

  • please give browser name, version, and operating system when reporting
    failures (and a stack trace if you know how to generate one)

I was using Chrome 22.0.1229.94 on Mac. I will try to test further, but
probably won't get to it until tomorrow or Sunday.

I tested this against Safari and Chrome, the error Brian is seeing doesn't
make sense to me, but I'm going to check this whole approach out with the
MathJax developers.

Thanks, that would be quite helpful.

Cheers,

Brian

A


Reply to this email directly or view it on GitHubhttps://github.com/ipython/ipython/pull/2517#issuecomment-9819019.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

Contributor

ahmadia commented Oct 26, 2012

@ellisonbg : Is it possible to create a PR that removes the references and line numbers
but keeps the general approach to MathJax+Markdown processing?

Yes, I'll work on that now. Just a note, we have identical versions of Chrome and OS, and neither David or I saw the issue you mentioned. Are you working from a locally installed MathJax? Did you, ahem, clear your cache? I think it's worth reverting the code for now anyway, and there are definitely problems I'd like to pester Davide Cervone about.

Contributor

ahmadia commented Oct 28, 2012

Okay, I've thought about this a little more, and read through the MathJax documentation and mailing list discussions on this issue more carefully.

I thought I would be able to get away with re-rendering the entire page every time a cell was edited, but now it's clear that for performance reasons we need to should only reprocess the edited div.

I think we can add equation numbering back in, but it will need to be a separate action triggered by dropdown menu/button/command key because the entire page must be re-rendered from scratch when this is requested.

ketch commented Oct 29, 2012

I think we can add equation numbering back in, but it will need to be a separate action triggered by dropdown menu/button/command key because the entire page must be re-rendered from scratch when this is requested.

As someone who wants this feature very much, I think that would be a perfectly acceptable setup. Equation numbering is mainly for readers; you don't need it much when you are writing a notebook, so I wouldn't expect this to be much of a bother.

Contributor

ahmadia commented Oct 29, 2012

@ketch @ellisonbg - I found and fixed a couple mistakes I had made with regard to development, and I backed out the equation numbers/references. Please pull this branch and test it. Please also test with your internet connection turned off (with/without a local mathjax copy). Offline --no-mathjax notebooks are currently broken in master (sorry!).

Side note about re-enabling equation numbers/references

@ketch - if you'd like to turn equation numbers/references on, the following seems to work for me:

Modify the IPython/frontend/html/notebook/static/js/mathjaxutils.js file like this to turn on equation numbers/references in the MathJax configuration by adding the line starting with TeX. You'll also notice that you can center your equations here if you'd prefer :)

    MathJax.Hub.Config({
        TeX: { equationNumbers: { autoNumber: "AMS", useLabelIds: true } },
        tex2jax: {
            inlineMath: [ ['$','$'], ["\\(","\\)"] ],
            displayMath: [ ['$$','$$'], ["\\[","\\]"] ],
            processEnvironments: true
        },
        displayAlign: 'left', // Change this to 'center' to center equations.
        "HTML-CSS": {
            styles: {'.MathJax_Display': {"margin": 0}}
        }
        });

Equation numbers/references will have a decent chance at getting rendered correctly, just remember to throw equations in align or equation environments (the MathJax examples uses a few shortcuts) if you want to attach references to them. If things start to render improperly, save the notebook, then reload it manually with your browser, and everything should render correctly. This is very beta functionality, but I know you're willing to deal with the annoyance of a browser refresh now and then for nicer typesetting. I'm interested in seeing how you are using equation numbers/references so I can plan around your use cases for future development on this code.

Contributor

ahmadia commented Oct 29, 2012

(I also just rewrote history very slightly and amended the commit message). Placeholders are not being rendered, this is a very minor fix to be made before merging into master.

ketch commented Oct 30, 2012

I confirm that the rendering speed is back to normal with this fix, and it works both with local and online MathJax. Also, the --no-mathjax option is fine now.

@ellisonbg ellisonbg commented on an outdated diff Oct 31, 2012

IPython/frontend/html/notebook/static/js/cell.js
Cell.prototype.typeset = function () {
- if (window.MathJax){
@ellisonbg

ellisonbg Oct 31, 2012

Owner

How do code cells get typeset now?

@ellisonbg ellisonbg commented on an outdated diff Oct 31, 2012

IPython/frontend/html/notebook/static/js/mathjaxutils.js
// MathJax loaded
- MathJax.Hub.Config({
@ellisonbg

ellisonbg Oct 31, 2012

Owner

Can you describe why it is needed to move the config from here into a <srcript> tag in the html?

@ellisonbg ellisonbg commented on an outdated diff Oct 31, 2012

IPython/frontend/html/notebook/static/js/mathjaxutils.js
@@ -102,6 +89,7 @@ IPython.mathjaxutils = (function (IPython) {
// math, then push the math string onto the storage array.
// The preProcess function is called on all blocks if it has been passed in
var process_math = function (i, j, pre_process) {
+ var HUB = MathJax.Hub;
@ellisonbg

ellisonbg Oct 31, 2012

Owner

Let's call this hub.

@ellisonbg ellisonbg commented on an outdated diff Oct 31, 2012

IPython/frontend/html/notebook/static/js/textcell.js
@@ -40,6 +40,7 @@ var IPython = (function (IPython) {
onKeyEvent: $.proxy(this.handle_codemirror_keyevent,this)
});
// The tabindex=-1 makes this div focusable.
+ // id is a unique cell_id necessary for updating MathJax intelligently
@ellisonbg

ellisonbg Oct 31, 2012

Owner

I think this comment is no longer applicable.

@ellisonbg ellisonbg commented on an outdated diff Oct 31, 2012

IPython/frontend/html/notebook/static/js/textcell.js
@@ -77,6 +78,13 @@ var IPython = (function (IPython) {
return false;
};
+ TextCell.prototype.typeset = function () {
@ellisonbg

ellisonbg Oct 31, 2012

Owner

When is this called? It seems like this typesets without the needed latex replacement logic.

Owner

ellisonbg commented Oct 31, 2012

I have tested all possible combinations (Local/CDN, --no-mathjax) and all issues appear to be fully resolved. Thanks for chasing all of this down. Please see my comments on the code above though.

If I understand things correctly, the current version won't number equations or do reference, but if we change the Math Jax config, it would work each time the user reloads the page.

ahmadia added some commits Oct 25, 2012

@ahmadia ahmadia Improved MathJax, missing callback workaround
MathJax.InputJax.TeX.resetEquationNumbers is supposed to be available,
but isn't consistently loaded before the notebook fires off its first
re-render request.  The code edits in mathjaxutils.js fix this issue.

Additionally, there was some init code in mathjaxutils.js that has been
properly excised to the html templates.

Removed some orphan rendering code so that all typesetting now goes
through mathjaxutils.js

Finally, removed an extra cell in the demo notebook.
9e186f7
@ahmadia ahmadia Remove Equation References/Numbering, Fix Bugs
Equation References and Numbering are not going to be trivial to add,
so the code has been removed for now.  Important fixes include
no-MathJax support (previously, the code was failing), and the
generation of unique ids for the rendering content in each cell,
tremendously speeding up MathJax rendering.

I am still not rendering placeholder text.
e2f6593
@ahmadia ahmadia undid unique ids, used jQuery 022386f
Contributor

ahmadia commented Nov 1, 2012

@ellisonbg - Thanks for the detailed code review. I'm pushing changes according to your suggestions, so I will respond to your comments in summary form below.

Cell.prototype.typeset - Why the move to TextCell? Is it safe to call this function without the markup fixes?

The move to TextCell was motivated by trying to trace the rendered div in the DOM. I don't need to do this anymore, so I restored the typeset functionality back to the Cell class. As for markup fixes, these are only needed for cells that use the Markdown parser. You'll notice that the typeset call in the Markdown.prototype.render function is performed after all the math-wrangling trickery.

Moving the MathJax configuration to the static .html files

When I was trying to figure out why equation referencing was subtly broken, I assumed part of the problem was that I wasn't ensuring that MathJax was being loaded early enough (the documentation suggests including MathJax in this way, but it is not mandatory). It turns out that the difference is actually between using:

{{mathjax_url}}?config=TeX-AMS_HTML and
{{mathjax_url}}?config=TeX-AMS_HTML-full

According to the MathJax documentation, we shouldn't configure after MathJax is loaded unless we use the &delayStartupUntil=configured parameter and later call MathJax.Hub.Configured(). This was working for me anyway in Chrome, but I suspect there are other browsers where this might be a problem. I've moved the configuration from the .html files back to the .js and I have added the delayStartup parameter for correctness.

I've also rewritten my comment above to reflect the file to modify to enable equation numbering/references.

Renaming HUB to hub

Done.

Vestigial comment about id

Removed.

@ahmadia ahmadia Finalizing fixes to MathJax enhancements
* Cell.prototype.typeset functionality restored
* MathJax configuration files back in .js, delayedStartup
* Renamed HUB to hub
* Removed vestigial comment
* Restored Markdown Cell placeholder render functionality
87b7b8a
Owner

ellisonbg commented Nov 1, 2012

So I think this branch is looking good, but I would like it if some others could test it. But Aron, this is great work, we really appreciate your chasing down all of these things.

ketch commented Nov 2, 2012

Works for me with the new commits. It might even be faster (rendering) than 0.13 now; it's hard to tell because they're both pretty fast on my worksheets.

Owner

fperez commented Nov 2, 2012

I've tested it with many of our examples that were basically unusable. @ahmadia, many thanks for tracking these problems down, and everyone else for the careful testing and review!

Because this is so high priority (master is basically unusable right now with large/complex notebooks), I'm going to merge this now. We can always refine further, but having an effectively unusable master isn't a good idea.

fperez merged commit 2962dae into ipython:master Nov 2, 2012

ahmadia deleted the ahmadia:mathjax_fix2 branch May 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment