julou [julou]

Hi julou,

Thanks for your initial work, there are a few problems with your module:

a) Please don't use $(document).ready, use Drupal.behaviours for adding javascript functionality.
b) I'm not sure why you're using hook_preprocess ... I think hook_init is probably a better choice, then all you'd need is the two drupal_add_js lines.
c) Please use the Libraries API for installing and downloading the MathJAX library, http://drupal.org/project/libraries

If you could make the above changes, the module is great and I'm looking forward to your next version.

Hello, and thank you for applying for a CVS account.

As per requirements, the motivation needs to include more than two sentences (possibly a paragraph) that describe the module features (the description should make clear what the module features are without to look at the code), and a comparison with the existing solutions.

I will report here a little checklist:

• The code needs to follow the coding standards; check in particular the format used for the control structures, and the name given to PHP variables, Drupal persistent variables, functions defined from the module.
• Files available from third-party sites should not be included within the module/theme. This is particularly true for files that are not licensed under GPL License v2, but it is also true for files that are licensed under the same license used by Drupal.
• The license file should not be included as well; the packaging script already include that file. In any cases, the code for modules/themes committed in drupal.org repository needs to be released under the same license used by Drupal; any compatible license is not allowed.
• Check the code passes the Coder validation.

Hi julou,

Getting better, but you've still got some things to do!
- Try hook_init with if (arg(0) === 'node'), it really doesn't make sense to be using preprocess there
- Add a hook_requirements check for the mathjax library (I think the library name should be lowercase)
- Extra points: add a drush command to download the mathjax library (like the ckeditor module)

Thanks and keep up the good work.

Yes, you can check what node type is being used with if (node_load(arg(1))->type === 'page') { ...

The arguments come from the url, so yoursite.com/node/1/edit would have arg(0) = node, arg(1) = 1, arg(2) = edit. This is true even the page is aliased.

A user profile page is /user/1 so you can check for this with arg(0) = user, numeric(arg(1)) and !arg(2).

If you're doing these checks you might want to add an administration page where you can control the choices, it will help meat out your module too.

Looks great, I'm happy. kiamlaluno has the final say here :)

There are just two things that should be changed:

• t() is not used inside the installation file; get_t() should be used in hook_requirements(), and the strings should not be translated in the hook_update_N() implementations.
• There should be a jQuery function to create HTML tags in the DOM.

• The correct sentence is t() is not used in an installation file; I specified what should be done in hook_requirements(), and outside hook_requirements() to avoid the misunderstanding that get_t() should be used in all the code contained in the installation file.
• The JavaScript code contains a document.createElement(); the jQuery library has its own manipulation functions.

I think the hook_requirements check should be a runtime check (move it into the .module file) which also moots the get_t problem :)

Also, I think, the document.createElement is fine - it's for asynchronous loading rather than standard manipulation, and it's a straight copy of the JS recommended by MathJax - I think it makes sense to keep that standard instead of maintaining our own.

Thanks for running the gauntlet julou, looking forward to good things :)

hook_requirements() is in the installation file as it should; see what is done from Drupal core modules.

I am giving credits to the users who participated in this issue.