Comments

• 

  Anthony Borrow

  Wed, 21 Jan 2015, 3:27 PM

  Many thanks for sharing this plugin with the Moodle community.
  
  When adding the block to a course I noticed:
  
  Notice: Undefined property: block_searchonmath::$get_content in /var/www/m28/blocks/searchonmath/block_searchonmath.php on line 62
  
  Also, you have it set that the block has configuration settings but there is settings.php file.
  
  My largest concern is that from what I can tell it does not really seem to integrate any functionality into Moodle. (see https://docs.moodle.org/dev/Plugin_validation#Manual_checks (7th bullet point).
  
  Also, it is really helpful if you could provide the requested meta-data that allows the rest of the community to participate with the code and giving feedback. Please review https://docs.moodle.org/dev/Plugin_contribution_checklist#Meta-data and then update the Moodle plugins entry. For now, I am going to mark this plugin as needing more work until we get these issues resolved. Once you get things tidied up, please reschedule the plugin for review and we can take another look at it. Thanks for your patience with the review and approval process. Peace - Anthony
• 

  SearchOnMath Company

  Wed, 28 Jan 2015, 12:59 AM

  Thank you for the quick reply!
  All meta-data is now available.
  
• 

  Plugins bot

  Thu, 29 Jan 2015, 5:22 PM

  Prechecker results: http://integration.moodle.org/job/Precheck%20remote%20branch/10242/artifact/work/smurf.html
• 

  David Mudrák

  Thu, 29 Jan 2015, 5:31 PM

  Thanks for sharing this plugin with the Moodle community. I can see that some of the issues mentioned below are already addressed in
  your github repository. Please note that we do not automatically pull updates from github. You are supposed to release new versions
  by uploading them into the plugins directory.
  
  * Please fix the GPL boilerplate at the top of your files. It is supposed to be a comment inside the PHP block. You currently have
  it as HTML comments which puts it into the generated HTML code of the page and may break the output of Moodle command line
  utilities. See https://docs.moodle.org/dev/Coding_style#Files
  * Please see https://docs.moodle.org/dev/jQuery for the recommended way to include jQuery in plugins (again, putting script into the
  top level of your block file may lead to unexpected behaviour.
  * You have some strings hard-coded in English.
  * The files README.md and Description.txt seem to be overlapping. Can you please the information from both into the README file and
  remove the other?
  * We have a policy that plugins requiring uncommon post-installation tweaks are moved into the Experimental category here in the
  plugins directory. A specific MathJax configuration is not exactly that case as long as your block copes well with the
  situation that MathJax is either disabled or not configured exactly as you expect. Also, can you please describe the required
  setup in the plugin description here? Some admins may prefer installation via the web UI and they might not have easy access to
  the README file. That is why we generally suggest to keep the information in the README and in the plugin description in sync.
  
  I am going to mark this as needing more work so we can get these issues fixed. Thanks for your patience.
  
• 

  SearchOnMath Company

  Fri, 6 Feb 2015, 7:13 AM

  Thank you for the reply. We hope that there are no more issues now.
• 

  David Mudrák

  Sun, 8 Feb 2015, 5:14 AM

  Hi. Thanks for providing the new version. The good news is that the plugin does not break CLI utilities any more, as it did. The not-so-good news is that I am completely confused about the plugin now (although I may be missing something indeed).
  
  1. Your description and the README file suggests that special configuration of the MathJax is needed. However, from playing with the plugin and looking at its code, it does not seem to have any relation to MathJax at all. Your block puts an inline JS that searches for all tags with the class "texrender". But this is not what the MathJax filter produces. The output of the MathJax filter is HTML/CSS and MathML. Images you are working with are created by the TeX filter and I am indeed able to test your plugin with default TeX filter enabled and MathJax disabled (thence my confusion).
  
  2. Beside that, the so called required MathJax configuration in your description is actually the default configuration in Moodle 2.8 (apart from the fact that your README and description contain a typo - missing ); - that causes the filter not to work at all.
  
  3. You may want to check http://security.stackexchange.com/questions/66252/encodeuricomponent-in-a-unquoted-html-attribute for potential security related discussion related to your implementation.
  
  After all, I don't even think that a block is the best way how this functionality should be implemented. Why not to make it as another filter that would simply extend what the other filter (TeX in your case) created? The instructions would have to just highlight that your filter must be applied after the TeX filter and done. That would be way cleaner and you would also avoid the need for expensive regex search&replace in all the HTML code at every page request (which I am afraid is prone to false positive matches with negative consequences).
  
  I am going to mark this as needing more work so we can sort out these issues. Looking forward to hear more details and your reasoning for the current implementation. Thanks for your patience with the review process.
• 

  SearchOnMath Company

  Wed, 11 Feb 2015, 4:00 AM

  Thank you for the reply!
  
  1 - You are right. It works with MathJax disabled. The description will be updated.
  
  2 - That's odd. When I tested an early version of the plugin in a computer different of the one I'm using to develop it (using a Moodle 2.8), the default MathJax configuration didn't allow the plugin to work. It only worked after the configuration present in the description was added. Could it be a installation problem? I will reinstall Moodle on that computer just to check
  
  3 - I will take those potential issues in consideration. Thank you for the link!
  
  The block implementation was choosen because of two reasons:
  
  1 - It's a nice way to show a link direct to the SearchOnMath initial page.
  2 -A filter was the first option considered. However, we had some issues with Moodle caching and decided to use a block.
  
  If it's necessary, we can try the filter again.
  Thank you for the quick reply and the patience with the review process.
  
• 

  David Mudrák

  Thu, 12 Feb 2015, 5:29 AM

  Can we please have the description more useful and make it clear what filters and what order must be enabled on the site to make it work? I still don't understand why or how MathJax should/could be involved in this.
• 

  SearchOnMath Company

  Thu, 12 Feb 2015, 5:46 AM

  I'm sorry for the confusion, but MathJax is not necessary. Only the default Moodle TeX filter is necessary.
• 

  David Mudrák

  Thu, 12 Feb 2015, 10:33 PM

  Thanks. You are cleared to land now, welcome to the Plugins directory!
• 

  SearchOnMath Company

  Fri, 13 Feb 2015, 8:02 PM

  Thank you.