Details

    • Testing Instructions:
      Hide

      You can test in almost any instance of the editor, but so as to clarify, I suggest that you go to any course settings page and use the 'Course summary' editor at top.

      Except for an improvement in caching behaviour, this test script should have the same result after this change as before.

      PART ONE - ENABLING PLUGINS

      0. If necessary, edit filter settings in admin and enable the TeX and emoticon filters.
      1. Look at the editor.
      * Verify that it contains all of the custom plugin buttons: no-link, smiley, media, dragmath, and spellchecker.
      2. In a separate tab, open the Filters page from under Course administration in Settings block. Turn Emoticons off and save changes.
      3. Reload the course settings page.
      * The emoticons button should have disappeared, while the other three remain.
      4. In the filters page, turn emoticons on again but turn off TeX. Reload course settings page.
      * The equation button should now have disappeared, but the smiley is back.
      5. Now turn both filters back to Default (On) again and save ready for further testing.
      6. Go to the TinyMCE settings page in admin, temporarily remove (cut to clipboard) the 'spell language list', and save changes.
      7. Reload the course settings page.
      * The spell checker button should have disappeared.
      8. Put the spell language list back and save changes again.

      Note: The 'insert media' button (and no-link) is always on and does not depend on filters. This is how it worked before too.

      PART TWO - PLUGINS STILL ACTUALLY WORK

      1. Type 'This is a frog.' into the editor and double-click to select the word 'frog'.
      2. Click the no-link button.
      3. Click HTML button to see source code
      * Should be: <p>This is a <span class="nolink">frog</span>.</p>
      4. Cancel out of the HTML view, then click the 'insert media' button.
      5. Click 'Find or upload...' button.
      6. Select or upload an mp3 file.
      * Once selected, a preview player for the mp3 should appear.
      7. Play the mp3 file from the preview.
      * Check it plays OK.
      8. Click 'Insert'.
      * A link to your mp3 file, with the text being the name of the file, should be added into the editor.
      9. Click the smiley button.
      * A dialog showing lots of smileys should appear.
      10. Scroll down to bottom of dialog and have a look.
      * There should be a Cancel button.
      11. Mouse over to the 'big grin' smiley and click it.
      * While mousing over, a highlight bar should display.
      * When you click it, the dialog should disappear and the smiley should be inserted in your text.
      12. Click the equation button. (You may have to confirm a security prompt and wait a bit for Java to load.)
      * Dialog should pop up containing the Java equation editor
      13. Piddle about with the editor a bit until you get it to actually insert something.
      14. Click the Insert button.
      * TeX code representing your piddling about (surrounded by $$ markers) should be inserted into the editor.
      15. Delete editor content and type the text 'frog frogg'. Click the spell checker button.
      * The incorrectly spelled word should be underlined.
      16. Click on the incorrectly spelled word.
      * A suggestion list should appear.

      PART THREE - CACHING

      You will need to use a tool such as Firebug or equivalent in other browsers to monitor downloads during this part of the test. Instructions are written for Firebug.

      0. Go into your server settings and set debugging to 'Developer' level.
      1. Open Firebug 'Net' panel and switch it to the 'JS' tab.
      2. Go to a course settings page (this part is to make sure editor is cached as much as it will allow) then go to the course page, then go back to course settings.
      3. Look for four editor_plugin.js requests beginning with lib/editor/tinymce/plugins/loader.php/dragmath/-1/editor_plugin.js.
      * The requests should be shown in bold and showing a 'Remote IP' value, indicating that the file was actually requested and not cached.
      4. Right-click on one of the files and choose 'Open in new tab'.
      * The file should have normal formatting with multiple lines.
      5. Now change debugging to the level just below 'Developer' (ALL).
      6. Go to the course settings page (again, to make sure editor is cached) then go to the course page, then back to course settings.
      7. Look in Firebug for the four .js requests.
      * The requests should be HTTP 200 OK but grey and without any 'Remote IP' value, indicating that they were loaded from cache.
      * The first URL should now be something like lib/editor/tinymce/plugins/loader.php/dragmath/2012051700/editor_plugin.js (containing a version number).
      8. Right-click on one of the files and choose 'Open in new tab'.
      * The file should be minimised (on a single line).
      9. Close all windows of your browser completely.
      10. Open the browser again, go back to Moodle, and log in.
      11. Open Firebug 'Net' panel if needed and switch to 'JS' tab.
      12. Go to a course settings page.
      13. Look in net panel for the four .js requests.
      * The requests should still be grey HTTP 200 OK without a 'Remote IP' value, indicating that they are still cached even after the session ended and no server requests were made.

      Show
      You can test in almost any instance of the editor, but so as to clarify, I suggest that you go to any course settings page and use the 'Course summary' editor at top. Except for an improvement in caching behaviour, this test script should have the same result after this change as before. PART ONE - ENABLING PLUGINS 0. If necessary, edit filter settings in admin and enable the TeX and emoticon filters. 1. Look at the editor. * Verify that it contains all of the custom plugin buttons: no-link, smiley, media, dragmath, and spellchecker. 2. In a separate tab, open the Filters page from under Course administration in Settings block. Turn Emoticons off and save changes. 3. Reload the course settings page. * The emoticons button should have disappeared, while the other three remain. 4. In the filters page, turn emoticons on again but turn off TeX. Reload course settings page. * The equation button should now have disappeared, but the smiley is back. 5. Now turn both filters back to Default (On) again and save ready for further testing. 6. Go to the TinyMCE settings page in admin, temporarily remove (cut to clipboard) the 'spell language list', and save changes. 7. Reload the course settings page. * The spell checker button should have disappeared. 8. Put the spell language list back and save changes again. Note: The 'insert media' button (and no-link) is always on and does not depend on filters. This is how it worked before too. PART TWO - PLUGINS STILL ACTUALLY WORK 1. Type 'This is a frog.' into the editor and double-click to select the word 'frog'. 2. Click the no-link button. 3. Click HTML button to see source code * Should be: <p>This is a <span class="nolink">frog</span>.</p> 4. Cancel out of the HTML view, then click the 'insert media' button. 5. Click 'Find or upload...' button. 6. Select or upload an mp3 file. * Once selected, a preview player for the mp3 should appear. 7. Play the mp3 file from the preview. * Check it plays OK. 8. Click 'Insert'. * A link to your mp3 file, with the text being the name of the file, should be added into the editor. 9. Click the smiley button. * A dialog showing lots of smileys should appear. 10. Scroll down to bottom of dialog and have a look. * There should be a Cancel button. 11. Mouse over to the 'big grin' smiley and click it. * While mousing over, a highlight bar should display. * When you click it, the dialog should disappear and the smiley should be inserted in your text. 12. Click the equation button. (You may have to confirm a security prompt and wait a bit for Java to load.) * Dialog should pop up containing the Java equation editor 13. Piddle about with the editor a bit until you get it to actually insert something. 14. Click the Insert button. * TeX code representing your piddling about (surrounded by $$ markers) should be inserted into the editor. 15. Delete editor content and type the text 'frog frogg'. Click the spell checker button. * The incorrectly spelled word should be underlined. 16. Click on the incorrectly spelled word. * A suggestion list should appear. PART THREE - CACHING You will need to use a tool such as Firebug or equivalent in other browsers to monitor downloads during this part of the test. Instructions are written for Firebug. 0. Go into your server settings and set debugging to 'Developer' level. 1. Open Firebug 'Net' panel and switch it to the 'JS' tab. 2. Go to a course settings page (this part is to make sure editor is cached as much as it will allow) then go to the course page, then go back to course settings. 3. Look for four editor_plugin.js requests beginning with lib/editor/tinymce/plugins/loader.php/dragmath/-1/editor_plugin.js. * The requests should be shown in bold and showing a 'Remote IP' value, indicating that the file was actually requested and not cached. 4. Right-click on one of the files and choose 'Open in new tab'. * The file should have normal formatting with multiple lines. 5. Now change debugging to the level just below 'Developer' (ALL). 6. Go to the course settings page (again, to make sure editor is cached) then go to the course page, then back to course settings. 7. Look in Firebug for the four .js requests. * The requests should be HTTP 200 OK but grey and without any 'Remote IP' value, indicating that they were loaded from cache. * The first URL should now be something like lib/editor/tinymce/plugins/loader.php/dragmath/2012051700/editor_plugin.js (containing a version number). 8. Right-click on one of the files and choose 'Open in new tab'. * The file should be minimised (on a single line). 9. Close all windows of your browser completely. 10. Open the browser again, go back to Moodle, and log in. 11. Open Firebug 'Net' panel if needed and switch to 'JS' tab. 12. Go to a course settings page. 13. Look in net panel for the four .js requests. * The requests should still be grey HTTP 200 OK without a 'Remote IP' value, indicating that they are still cached even after the session ended and no server requests were made.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w33_MDL-33041_m24_tinymceplugins
    • Rank:
      40218

      Description

      (NOTE: As this is a new feature, it is probably appropriate for Moodle 2.4. The version is set to 2.3 because there isn't an entry for 2.4 yet.)

      At present there are 4 moodle core custom plugins inside TinyMCE. These are stored within the versioned TinyMCE folder, and hardcoded into the TinyMCE options in the lib.php file. This approach sucks because:

      • The Moodle plugins shouldn't really be stored inside the third-party library folder, which changes every time the TinyMCE version is updated.
      • It is impossible to include extra TinyMCE plugins in a Moodle installation without making a change to core code (lib.php), as well as putting your own plugins inside that third-party library folder.

      My proposal is as follows:

      • Create a plugins folder directly inside lib/editor/tinymce.
      • As editors cannot have subplugins (only modules can), the contents of this folder will not be standard Moodle plugins, but will be a lightweight plugin type defined and handled specially within editor/tinymce.
      • lib.php for these plugins contains a suitable plugin class which will be able to do the following:

      a) Modify the TinyMCE options (e.g. add or remove plugins from the list of buttons, in whatever row)
      b) Return details of their own TinyMCE plugin name and JavaScript file(s) so it can be passed as parameter to the M.editor_tinymce.init_editor or something, so that it can end up calling this function before tinyMCE.init (example from Tim's plugin):

      tinymce.PluginManager.load('supsub', M.cfg.wwwroot + '/lib/editor/supsub/supsub_plugin.js');
      
      • Move the existing Moodle TinyMCE plugins into this new framework and remove the hardcoded conditions (about whether to use them or not) from the lib.php so that they are equivalent level to any new custom plugins that users may add.

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          This would be really beneficial, thanks for the suggestion!

          Show
          Dan Poltawski added a comment - This would be really beneficial, thanks for the suggestion!
          Hide
          Davo Smith added a comment -

          It would be nice if this plugin framework also allowed you to create a small plugin to fully customise the tinymce layout.

          For at least one site I've been involved in, buttons, colours, fonts and other settings were extensively adjusted (via a small hook into the tinymce lib.php file). Being able to do that without any core modifications would be great (although that might interplay badly with other plugins that want to add buttons, if my plugin wanted to remove them again).

          Show
          Davo Smith added a comment - It would be nice if this plugin framework also allowed you to create a small plugin to fully customise the tinymce layout. For at least one site I've been involved in, buttons, colours, fonts and other settings were extensively adjusted (via a small hook into the tinymce lib.php file). Being able to do that without any core modifications would be great (although that might interplay badly with other plugins that want to add buttons, if my plugin wanted to remove them again).
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Sam,
          very good idea : I use several math TinyMce plugins that I need to reinstall each time I upgrade my Moodle.
          Aditionnally to modifying lib/editor/tinymce/lib.php, I also need to add the lang stings of my plugins to lib/editor/tinymce/lang/en/editor_tinymce.php and do that for each language, so it would be a lot better if I can have a lang folder in each plugin subfolder with all files for differents languages.

          Show
          Jean-Michel Vedrine added a comment - Hello Sam, very good idea : I use several math TinyMce plugins that I need to reinstall each time I upgrade my Moodle. Aditionnally to modifying lib/editor/tinymce/lib.php, I also need to add the lang stings of my plugins to lib/editor/tinymce/lang/en/editor_tinymce.php and do that for each language, so it would be a lot better if I can have a lang folder in each plugin subfolder with all files for differents languages.
          Hide
          Martin Dougiamas added a comment -

          You wouldn't set the affects version to 2.4 anyway. Current settings are correct.

          Show
          Martin Dougiamas added a comment - You wouldn't set the affects version to 2.4 anyway. Current settings are correct.
          Hide
          Petr Škoda added a comment -

          hi,

          I was thinking about this too recently:

          • M.cfg.wwwroot + '/lib/editor/supsub/supsub_plugin.js' would fail badly during upgrades (caching troubles for some ppl), you should load all JS and CSS from some versioned directory the same way we do with tinymce itself
          • Another issue is we would need to inject all your plugin strings to the lang loader script. Maybe we would need some editor subplugin infrastructure for this too because otherwise AMOS could not deal with your lang strings - full plugins might be the only way to go.
          • alternatively we could load the whole tinymce via some loader which adds proper caching headers - the mixing of PHP and JS is a pain there at the moment.

          My +10 to get this implemented early in 2.4dev, I am sure the maths mob would not want to lynch us any more if we added this improvement. Another thing we could add is switching of editors on the fly, but it is not very related apart from the necessary init changes...

          Show
          Petr Škoda added a comment - hi, I was thinking about this too recently: M.cfg.wwwroot + '/lib/editor/supsub/supsub_plugin.js' would fail badly during upgrades (caching troubles for some ppl), you should load all JS and CSS from some versioned directory the same way we do with tinymce itself Another issue is we would need to inject all your plugin strings to the lang loader script. Maybe we would need some editor subplugin infrastructure for this too because otherwise AMOS could not deal with your lang strings - full plugins might be the only way to go. alternatively we could load the whole tinymce via some loader which adds proper caching headers - the mixing of PHP and JS is a pain there at the moment. My +10 to get this implemented early in 2.4dev, I am sure the maths mob would not want to lynch us any more if we added this improvement. Another thing we could add is switching of editors on the fly, but it is not very related apart from the necessary init changes...
          Hide
          Sam Marshall added a comment -

          Davo: You can do this with my framework as implemented - basically plugins get called after the $params array is built, and can change anything in it. So if you set your plugin sort order to a big number so it runs last, you can then fiddle around with all the options.

          Jean-Michel: That is definitely an important goal - although just moving plugins outside of the TinyMCE 3.5 folders is a good thing (already achieved in my current code), an even better one is making plugins independent and obviously, a plugin is not independent if its strings still have to go in the core lang file. (Note: It isn't possible to add strings in local lang files, so those don't even provide a workaround.)

          Unfortunately because moodle subplugins are only supported within modules, and the editor is not a module, it is currently not possible to use 'real' lang strings. I am currently not sure about the best solution to this. Basically there are two parts to the problem:

          1) Find some way to get lang files supported within these plugins so that I can write php code to retrieve the strings (whether that be standard get_string or some kind of hack which might have limitations, but it should at least let you put the file inside lang/en inside the plugin).

          2) Figure out how to get those strings over into TinyMCE JavaScript (it is already integrated somehow with Moodle lang system, just need to hook into this).

          I haven't solved this yet, but going to push my work-in-progress code (pretty slick I think which does everything else so people can see it if interested. Hopefully will get back to it tomorrow and have another look at the lang problem.

          Show
          Sam Marshall added a comment - Davo: You can do this with my framework as implemented - basically plugins get called after the $params array is built, and can change anything in it. So if you set your plugin sort order to a big number so it runs last, you can then fiddle around with all the options. Jean-Michel: That is definitely an important goal - although just moving plugins outside of the TinyMCE 3.5 folders is a good thing (already achieved in my current code), an even better one is making plugins independent and obviously, a plugin is not independent if its strings still have to go in the core lang file. (Note: It isn't possible to add strings in local lang files, so those don't even provide a workaround.) Unfortunately because moodle subplugins are only supported within modules, and the editor is not a module, it is currently not possible to use 'real' lang strings. I am currently not sure about the best solution to this. Basically there are two parts to the problem: 1) Find some way to get lang files supported within these plugins so that I can write php code to retrieve the strings (whether that be standard get_string or some kind of hack which might have limitations, but it should at least let you put the file inside lang/en inside the plugin). 2) Figure out how to get those strings over into TinyMCE JavaScript (it is already integrated somehow with Moodle lang system, just need to hook into this). I haven't solved this yet, but going to push my work-in-progress code (pretty slick I think which does everything else so people can see it if interested. Hopefully will get back to it tomorrow and have another look at the lang problem.
          Hide
          Sam Marshall added a comment -

          I've now added the code (see 'pull master diff URL'). After some initial twiddling, it's split into two main commits, one is the system and the other is moving the existing added plugins. So if you want to just look at the system, there's a pretty small commit to look at...

          Show
          Sam Marshall added a comment - I've now added the code (see 'pull master diff URL'). After some initial twiddling, it's split into two main commits, one is the system and the other is moving the existing added plugins. So if you want to just look at the system, there's a pretty small commit to look at...
          Hide
          Sam Marshall added a comment -

          Petr: I didn't see your comments before adding mine, sorry. Not ignoring them. Response to them now:

          1) Caching problems - Argh, forgot about that. Off top of my head I think it is OK to use a loader script instead of serving the files directly, loader script should have plugin's version.php number in the url, and can then set all the plugin files to expire:never except in developer debug mode, for good performance.

          2) Lang files & whether to make it allow proper subplugins inside editor - yes will look at this tomorrow, maybe that is easiest solution.

          Show
          Sam Marshall added a comment - Petr: I didn't see your comments before adding mine, sorry. Not ignoring them. Response to them now: 1) Caching problems - Argh, forgot about that. Off top of my head I think it is OK to use a loader script instead of serving the files directly, loader script should have plugin's version.php number in the url, and can then set all the plugin files to expire:never except in developer debug mode, for good performance. 2) Lang files & whether to make it allow proper subplugins inside editor - yes will look at this tomorrow, maybe that is easiest solution.
          Hide
          Petr Škoda added a comment -

          You would need to use something like tinynymce/plugins/moodleemoticon/resources/1212/editor_plugin_src.js and add the links from tinynymce/plugins/moodleemoticon/lib.php, the 1212 is version number which woudl be incremented after avery change.

          Show
          Petr Škoda added a comment - You would need to use something like tinynymce/plugins/moodleemoticon/resources/1212/editor_plugin_src.js and add the links from tinynymce/plugins/moodleemoticon/lib.php, the 1212 is version number which woudl be incremented after avery change.
          Hide
          Petr Škoda added a comment -

          For the lang related code see timymce/extra/strings.php, it could go through all your subplugins and merge it in.

          Show
          Petr Škoda added a comment - For the lang related code see timymce/extra/strings.php, it could go through all your subplugins and merge it in.
          Hide
          Petr Škoda added a comment -

          I tried to use loader for tinymce and failed badly, the splitting of JS/CSS/image resources and serverside PHP scripts could help a lot there.

          Show
          Petr Škoda added a comment - I tried to use loader for tinymce and failed badly, the splitting of JS/CSS/image resources and serverside PHP scripts could help a lot there.
          Hide
          Sam Marshall added a comment -

          thanks for explanation re lang file.

          Re loader, I was thinking of like:

          tinymce/plugins/loader.php/moodleemoticon/2012070400/editor_plugin.js

          That way it can access all the resources the same way without me having to actually change any of the plugin code (which uses relative paths from the .js file). There is a bit in the plugin code where it puts the path to the js file into the params array for tinymce to load it, so would just need to add version there.

          I would need to make a special case so that for .php files (again the plugins use relative path to these), it just calls require on the original .php script.

          Of course will be careful about security problem in this script (no access outside folder, etc). Think it should work though, I'll give it a go tomorrow.

          Show
          Sam Marshall added a comment - thanks for explanation re lang file. Re loader, I was thinking of like: tinymce/plugins/loader.php/moodleemoticon/2012070400/editor_plugin.js That way it can access all the resources the same way without me having to actually change any of the plugin code (which uses relative paths from the .js file). There is a bit in the plugin code where it puts the path to the js file into the params array for tinymce to load it, so would just need to add version there. I would need to make a special case so that for .php files (again the plugins use relative path to these), it just calls require on the original .php script. Of course will be careful about security problem in this script (no access outside folder, etc). Think it should work though, I'll give it a go tomorrow.
          Hide
          Petr Škoda added a comment -

          1/ I really do not like the proposed mixing of static tinymce stuff (js,images,css) with standard moodle plugin files, could we please make a special subdirectory for it? (tinymce, static, resources or whatever else).

          2/ proxying of php files seems crazy to me - there is imo no problem in using absolute paths in the plugin JS code (or relative to main tinymce script), we do not know if all 3rd party scripts are going to support it, upstream tinymce does not have php (only the spellchecker extra which is not part of their distro I think), it would unnecessarily complicate the loader.

          3/ I like the plugin version numbers use in loader url!

          4/ I am staring to hate all the separate options like $cachejs, comboloader, theme designer mode etc. (I introduced most of them myself) - we would need one more for this, maybe alternatively we could create a new $CFG->optimisestaticserving and remove the rest...

          5/ It seems you are trying hard to push this through to 2.3dev, I am just a bit afraid that we would have to change the apis once again in 2.4 when implementing user and sitewide editor preferences (enabling/disabling and ordering of tinymce plugins, switching of editors), on the other hand if Martin in 2.3 wants this I am sure we could get it ready for integration by the end of next week.

          In any case thanks a lot for bringing this forward, we have independently came to very similar solutions.

          Show
          Petr Škoda added a comment - 1/ I really do not like the proposed mixing of static tinymce stuff (js,images,css) with standard moodle plugin files, could we please make a special subdirectory for it? (tinymce, static, resources or whatever else). 2/ proxying of php files seems crazy to me - there is imo no problem in using absolute paths in the plugin JS code (or relative to main tinymce script), we do not know if all 3rd party scripts are going to support it, upstream tinymce does not have php (only the spellchecker extra which is not part of their distro I think), it would unnecessarily complicate the loader. 3/ I like the plugin version numbers use in loader url! 4/ I am staring to hate all the separate options like $cachejs, comboloader, theme designer mode etc. (I introduced most of them myself) - we would need one more for this, maybe alternatively we could create a new $CFG->optimisestaticserving and remove the rest... 5/ It seems you are trying hard to push this through to 2.3dev, I am just a bit afraid that we would have to change the apis once again in 2.4 when implementing user and sitewide editor preferences (enabling/disabling and ordering of tinymce plugins, switching of editors), on the other hand if Martin in 2.3 wants this I am sure we could get it ready for integration by the end of next week. In any case thanks a lot for bringing this forward, we have independently came to very similar solutions.
          Hide
          Sam Marshall added a comment -

          1/ Sure.

          2/ You're right it's probably a bad idea. I was trying to avoid changing any of the javascript code at all, because I do not know how to make the minified JS files that are in there... I'll find out and then it's OK to put the php files in different place. (In fact, probably this loader can do/cache the minimised versions to temp, which would save having those files in source tree at all, I assume the JS minimiser is in moodle somewhere.)

          3/ yay

          4/ Agree I hate those options but actually if you turn them all off it makes it kind of slow (string cache I think particularly) so I don't know what the right thing to do is... It would be nice if those options automatically only apply if you set debugging = developer (so you can just toggle debugging to see effect with/without).

          5/ I am not pushing this for 2.3 dev personally, 2.4 is fine by me. (Of course if somebody wants it in 2.3 that's okay too, if I get it finished soon.) I happen to be developing this now because of an internal task, but actually for the internal task we will hack current 2.2 core code, I just want an expiry date for the hack plus Dan asked me to propose this properly.

          Show
          Sam Marshall added a comment - 1/ Sure. 2/ You're right it's probably a bad idea. I was trying to avoid changing any of the javascript code at all, because I do not know how to make the minified JS files that are in there... I'll find out and then it's OK to put the php files in different place. (In fact, probably this loader can do/cache the minimised versions to temp, which would save having those files in source tree at all, I assume the JS minimiser is in moodle somewhere.) 3/ yay 4/ Agree I hate those options but actually if you turn them all off it makes it kind of slow (string cache I think particularly) so I don't know what the right thing to do is... It would be nice if those options automatically only apply if you set debugging = developer (so you can just toggle debugging to see effect with/without). 5/ I am not pushing this for 2.3 dev personally, 2.4 is fine by me. (Of course if somebody wants it in 2.3 that's okay too, if I get it finished soon.) I happen to be developing this now because of an internal task, but actually for the internal task we will hack current 2.2 core code, I just want an expiry date for the hack plus Dan asked me to propose this properly.
          Hide
          Petr Škoda added a comment -

          2/ no worries - the JS minimisation is done by java compressor during building of tinyMCE, I think we can switch to moodle integrated minify in loader and keep everything uncompressed in these new plugins, let me handle that later

          5/ maybe we could add subplugin support for text editors in 2.3 so that you do not have to hack anything outside of lib/editor/tinymce in OU code

          Show
          Petr Škoda added a comment - 2/ no worries - the JS minimisation is done by java compressor during building of tinyMCE, I think we can switch to moodle integrated minify in loader and keep everything uncompressed in these new plugins, let me handle that later 5/ maybe we could add subplugin support for text editors in 2.3 so that you do not have to hack anything outside of lib/editor/tinymce in OU code
          Hide
          Sam Marshall added a comment -

          Okay, new super wonderful finished version is done Not ready to submit for review yet as I still need to write a test script (so I might also find things wrong), but I have pushed the code just in case.

          Some notes:

          1. In a separate commit at beginning, I added subplugin support for editors. Don't think this will be a big performance concern as there are only two editors. So the TinyMCE subplugins are now proper Moodle subplugins.

          2. Each one can have its own lang file and all strings in that lang file are automatically available to TinyMCE via extras/strings.php. I moved the existing strings for 4 standard added plugins from the main lang file into these separate files (using AMOS too).

          3. The TinyMCE-specific static files are now included in a folder called 'tinymce' inside each plugin. (Other files are lib.php, version.php, and lang folder.)

          4. I made a loader.php to serve these static files. The files are served with names like, blah/loader.php/3.5_2012053100/my_plugin.js where 3.5 is the TinyMCE version and the other number is version.php for the subplugin. (It will shortly become obvious why I had to include the TinyMCE version which wasn't in the original plan.) Files are set to be cached by browser for one year.

          5. If you are in developer debug mode (I didn't make a separate setting - there is other similar logic that also uses this as a switch, and it doesn't totally kill performance, so probably okay) then it puts 'dev' into the URL in place of the version numbers above, and these files are then served with normal 'do not cache at all ever' Moodle headers.

          6. All JavaScript files served by the loader are minified and cached to temp folder using the functions in jslib.php, except when using 'dev' mode. (I removed the minified files from the source tree.)

          7. HTML files are token-replaced so that %%TINYMCEROOT%% points to the current TinyMCE version root directory. Plugins previously could refer to this within HTML files as '../../' so I needed to make another way to access it and there is no relative path to it because it depends on TinyMCE version now that these plugins are outside of the '3.5' directory. (Usage example is moodlemedia.htm.) Because str_replace is very fast and these files are still set to never expire, there is no point cacheing it, so this part was easy.

          8. I had to make a few other changes to the plugin files where plugins reference PHP scripts; basically the URL to a php script in the plugin main directory for a subplugin called frog is now url + '/../../../frog/script.php', where url is the URL of plugin location which gets passed into the plugin (see dragmath/tinymce/editor_plugin.js line 21).

          Show
          Sam Marshall added a comment - Okay, new super wonderful finished version is done Not ready to submit for review yet as I still need to write a test script (so I might also find things wrong), but I have pushed the code just in case. Some notes: 1. In a separate commit at beginning, I added subplugin support for editors. Don't think this will be a big performance concern as there are only two editors. So the TinyMCE subplugins are now proper Moodle subplugins. 2. Each one can have its own lang file and all strings in that lang file are automatically available to TinyMCE via extras/strings.php. I moved the existing strings for 4 standard added plugins from the main lang file into these separate files (using AMOS too). 3. The TinyMCE-specific static files are now included in a folder called 'tinymce' inside each plugin. (Other files are lib.php, version.php, and lang folder.) 4. I made a loader.php to serve these static files. The files are served with names like, blah/loader.php/3.5_2012053100/my_plugin.js where 3.5 is the TinyMCE version and the other number is version.php for the subplugin. (It will shortly become obvious why I had to include the TinyMCE version which wasn't in the original plan.) Files are set to be cached by browser for one year. 5. If you are in developer debug mode (I didn't make a separate setting - there is other similar logic that also uses this as a switch, and it doesn't totally kill performance, so probably okay) then it puts 'dev' into the URL in place of the version numbers above, and these files are then served with normal 'do not cache at all ever' Moodle headers. 6. All JavaScript files served by the loader are minified and cached to temp folder using the functions in jslib.php, except when using 'dev' mode. (I removed the minified files from the source tree.) 7. HTML files are token-replaced so that %%TINYMCEROOT%% points to the current TinyMCE version root directory. Plugins previously could refer to this within HTML files as '../../' so I needed to make another way to access it and there is no relative path to it because it depends on TinyMCE version now that these plugins are outside of the '3.5' directory. (Usage example is moodlemedia.htm.) Because str_replace is very fast and these files are still set to never expire, there is no point cacheing it, so this part was easy. 8. I had to make a few other changes to the plugin files where plugins reference PHP scripts; basically the URL to a php script in the plugin main directory for a subplugin called frog is now url + '/../../../frog/script.php', where url is the URL of plugin location which gets passed into the plugin (see dragmath/tinymce/editor_plugin.js line 21).
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Sam,
          I donwloaded your code and in very little time I converted one of my plugins. All seems to work well including languages files. This is wonderfull because it would remove my last patches of Moodle core files and make upgrading my Moodle a lot easier.

          Show
          Jean-Michel Vedrine added a comment - Hello Sam, I donwloaded your code and in very little time I converted one of my plugins. All seems to work well including languages files. This is wonderfull because it would remove my last patches of Moodle core files and make upgrading my Moodle a lot easier.
          Hide
          Jean-Michel Vedrine added a comment -

          Sam,
          After my test to create a new editor plugin, I tested the 4 ones included in core. No apparent problem for moodlenolink and moodlemedia but for Dragmath the insert and cancel buttons do nothing and for moodleemoticon the cancel button is labeled

          {#cancel}

          and I am unable to click on any emoticon in the plugin window.
          But I am not quite sure the problem is related to my setup, to recent changes in 23dev or to your changes ??

          Show
          Jean-Michel Vedrine added a comment - Sam, After my test to create a new editor plugin, I tested the 4 ones included in core. No apparent problem for moodlenolink and moodlemedia but for Dragmath the insert and cancel buttons do nothing and for moodleemoticon the cancel button is labeled {#cancel} and I am unable to click on any emoticon in the plugin window. But I am not quite sure the problem is related to my setup, to recent changes in 23dev or to your changes ??
          Hide
          Jean-Michel Vedrine added a comment -

          Hello,
          I have some problems with my own plugins and as they seems related to the problems I have with moodleemoticon I think it would be usefull to post
          I don't know how to get one of my plugin strings from a php file (but I have no problem getting the strings from the plugin js files). Unfortunately when I looked at what is in lib/editor/tinymce/plugins/moodleemoticon/dialog.php I see print_string('moodleemoticon:desc', 'editor_tinymce');
          But this doesn't work because if I look at the source code of the emoticon window on the test site when I have installed your code, I see the error message :
          Invalid get_string() identifier: 'moodleemoticon:desc' or component 'editor_tinymce'. Perhaps you are missing $string['moodleemoticon:desc'] = ''; in xxxx/lib/editor/tinymce/lang/en/editor_tinymce.php ?
          Also the problem with the cancel button is clearly the line :
          <input type="button" id="cancel" name="cancel" value="

          {#cancel}" onclick="tinyMCEPopup.close();" />
          because if I am not too confused {#cancel}

          syntax can't work here

          Show
          Jean-Michel Vedrine added a comment - Hello, I have some problems with my own plugins and as they seems related to the problems I have with moodleemoticon I think it would be usefull to post I don't know how to get one of my plugin strings from a php file (but I have no problem getting the strings from the plugin js files). Unfortunately when I looked at what is in lib/editor/tinymce/plugins/moodleemoticon/dialog.php I see print_string('moodleemoticon:desc', 'editor_tinymce'); But this doesn't work because if I look at the source code of the emoticon window on the test site when I have installed your code, I see the error message : Invalid get_string() identifier: 'moodleemoticon:desc' or component 'editor_tinymce'. Perhaps you are missing $string ['moodleemoticon:desc'] = ''; in xxxx/lib/editor/tinymce/lang/en/editor_tinymce.php ? Also the problem with the cancel button is clearly the line : <input type="button" id="cancel" name="cancel" value=" {#cancel}" onclick="tinyMCEPopup.close();" /> because if I am not too confused {#cancel} syntax can't work here
          Hide
          Sam Marshall added a comment -

          Thanks for testing Jean-Michel.

          You are right - I forgot to change the PHP code that gets those strings. It should be, like, get_string('desc', 'tinymce_moodleemoticon'). Also, I suspect there are some more path references (../ etc) that need to be changed.

          I'll work through testing and fixing all the 4 standard plugins (going to write a test script that covers it) and get back to you when they work, hopefully it will be clearer then.

          Show
          Sam Marshall added a comment - Thanks for testing Jean-Michel. You are right - I forgot to change the PHP code that gets those strings. It should be, like, get_string('desc', 'tinymce_moodleemoticon'). Also, I suspect there are some more path references (../ etc) that need to be changed. I'll work through testing and fixing all the 4 standard plugins (going to write a test script that covers it) and get back to you when they work, hopefully it will be clearer then.
          Hide
          Jean-Michel Vedrine added a comment -

          Thanks Sam,
          As expected 'desc', 'tinymce_moodleemoticon' was one of the few combinations I didn't try but as the plugin are now full subpugins it seems quite logical (easy for me to say that now !!). anyway, I have no problem with lang strings in my plugins now.
          yes, the problem with javascript seems like a path reference problem.
          Tell me if I can help you in testing.

          Show
          Jean-Michel Vedrine added a comment - Thanks Sam, As expected 'desc', 'tinymce_moodleemoticon' was one of the few combinations I didn't try but as the plugin are now full subpugins it seems quite logical (easy for me to say that now !!). anyway, I have no problem with lang strings in my plugins now. yes, the problem with javascript seems like a path reference problem. Tell me if I can help you in testing.
          Hide
          Jean-Michel Vedrine added a comment -

          I think the problem is the path to tiny_mce_popup.js

          Show
          Jean-Michel Vedrine added a comment - I think the problem is the path to tiny_mce_popup.js
          Hide
          Jean-Michel Vedrine added a comment -

          Also <script type="text/javascript" src="js/dialog.js?v=<?php echo $editor->version ?>"></script>
          shpuld be
          <script type="text/javascript" src="tinymce/js/dialog.js?v=<?php echo $editor->version ?>"></script>

          Show
          Jean-Michel Vedrine added a comment - Also <script type="text/javascript" src="js/dialog.js?v=<?php echo $editor->version ?>"></script> shpuld be <script type="text/javascript" src="tinymce/js/dialog.js?v=<?php echo $editor->version ?>"></script>
          Hide
          Petr Škoda added a comment -

          I have looked a bit at the code:

          1/ it is not compatible with slasharguments off - I think it should work the same way with and without the loader - %%TINYMCEROOT%% might be a dead end imo, it should use some JS function to get it consistently (or even directly M.cfg.wwwroot; without slasharguments it should imo load the files directly via apache because relative links would not work; the question is how reliable we want it, but I guess those potential caching problems for ppl without slashrguments would be acceptable.

          2/ the loader.php must work 100% without cookies/sessions (otherwise caching is not full working), it would be better if it used only stuff from lib/configonlylib.php the same way theme loaders do (I could work on this myself later)

          3/ $CFG->dataroot . '/temp is forbidden for this - use new $CFG->cachedir (temp is for current request only, cache is for other requests; there is new $CFG->cachedir and $CFG->tempdir which allows you to redirect it to faster/local disk)

          4/ there is no need to deal with exact versions of plugins/tinymce in the loader - it's only purpose is to invalidate caches, ignore any version stuff in loader url - always cache/serve the latest info

          5/ other loaders usually use "-1" for dev - mixing strings and integers is usually problematic

          6/ the spellchecker plugin would be probably moved to new location too - it is from upstream too, but it is a separate project and the last php script there (I maintain our hacks in https://github.com/moodle/custom-tinymce_spellchecker_php), but this can definitely wait, I would like to to send the whole tiny_mce through a slashargument loader and get rid of the subdir versioning completely (the perf gains could be relatively big)

          In any case this is very very promising, great work!!

          Show
          Petr Škoda added a comment - I have looked a bit at the code: 1/ it is not compatible with slasharguments off - I think it should work the same way with and without the loader - %%TINYMCEROOT%% might be a dead end imo, it should use some JS function to get it consistently (or even directly M.cfg.wwwroot; without slasharguments it should imo load the files directly via apache because relative links would not work; the question is how reliable we want it, but I guess those potential caching problems for ppl without slashrguments would be acceptable. 2/ the loader.php must work 100% without cookies/sessions (otherwise caching is not full working), it would be better if it used only stuff from lib/configonlylib.php the same way theme loaders do (I could work on this myself later) 3/ $CFG->dataroot . '/temp is forbidden for this - use new $CFG->cachedir (temp is for current request only, cache is for other requests; there is new $CFG->cachedir and $CFG->tempdir which allows you to redirect it to faster/local disk) 4/ there is no need to deal with exact versions of plugins/tinymce in the loader - it's only purpose is to invalidate caches, ignore any version stuff in loader url - always cache/serve the latest info 5/ other loaders usually use "-1" for dev - mixing strings and integers is usually problematic 6/ the spellchecker plugin would be probably moved to new location too - it is from upstream too, but it is a separate project and the last php script there (I maintain our hacks in https://github.com/moodle/custom-tinymce_spellchecker_php ), but this can definitely wait, I would like to to send the whole tiny_mce through a slashargument loader and get rid of the subdir versioning completely (the perf gains could be relatively big) In any case this is very very promising, great work!!
          Hide
          Petr Škoda added a comment -

          hmm, version disclosure for subplugins should not be a problem imo here because most of the plugin code code is in JS which is served anyway, and I guess in majority of cases you could guess version of plugin php files by looking at content of plugin JS files. In any case it might be better to not rely on the actual value so that we can change it in the future if necessary.

          Show
          Petr Škoda added a comment - hmm, version disclosure for subplugins should not be a problem imo here because most of the plugin code code is in JS which is served anyway, and I guess in majority of cases you could guess version of plugin php files by looking at content of plugin JS files. In any case it might be better to not rely on the actual value so that we can change it in the future if necessary.
          Hide
          Sam Marshall added a comment -

          I pushed a new version now to address all of Petr's concerns. (Note I have not yet fixed the bugs Jean-Michel identified, so the plugins still don't actually work properly, I will get to those soon. Sorry for delays but I had Moodle 2.3 and internal OU stuff to work on, and I do want to get this in early in 2.4 but that still leaves plenty of time.)

          1/ I removed the %%TINYMCEROOT%%, made it work with slasharguments off, and made it use JavaScript (kind of icky javascript) to print out the script tags for including files from within TinyMCE, based on the TinyMCE base dir from the parent window.

          2/ It will be difficult to make it work without any moodle lib functions (you can't include filelib when using that configonlylib define). Left this unchanged for Petr to maybe look at at some later date. However I added the NO_MOODLE_COOKIES define (otherwise there are probably performance problems from using session).

          3/ Changed to use cachedir (apparently it was said in the developer meeting today that there weren't any api changes in 2.3, but...

          4/ Changed. I already did ignore the user input always, the reason for before was that I was concerned about maybe old versions in the cache. Checked and found out it clears the cache on every version update (of a tinymce plugin, for instance) so there is no need to store version in filename in cache.

          5/ OK, I think strings would be better, but no problem, changed to -1

          6/ Leaving this alone, too complicated for me.

          I also rebased on latest-ish master. (People are making massively more changes than usual, that editor file didn't change for ages, then suddenly after there's a code freeze it goes mental.

          Show
          Sam Marshall added a comment - I pushed a new version now to address all of Petr's concerns. (Note I have not yet fixed the bugs Jean-Michel identified, so the plugins still don't actually work properly, I will get to those soon. Sorry for delays but I had Moodle 2.3 and internal OU stuff to work on, and I do want to get this in early in 2.4 but that still leaves plenty of time.) 1/ I removed the %%TINYMCEROOT%%, made it work with slasharguments off, and made it use JavaScript (kind of icky javascript) to print out the script tags for including files from within TinyMCE, based on the TinyMCE base dir from the parent window. 2/ It will be difficult to make it work without any moodle lib functions (you can't include filelib when using that configonlylib define). Left this unchanged for Petr to maybe look at at some later date. However I added the NO_MOODLE_COOKIES define (otherwise there are probably performance problems from using session). 3/ Changed to use cachedir (apparently it was said in the developer meeting today that there weren't any api changes in 2.3, but... 4/ Changed. I already did ignore the user input always, the reason for before was that I was concerned about maybe old versions in the cache. Checked and found out it clears the cache on every version update (of a tinymce plugin, for instance) so there is no need to store version in filename in cache. 5/ OK, I think strings would be better, but no problem, changed to -1 6/ Leaving this alone, too complicated for me. I also rebased on latest-ish master. (People are making massively more changes than usual, that editor file didn't change for ages, then suddenly after there's a code freeze it goes mental.
          Hide
          Petr Škoda added a comment -

          lol, thanks a lot! I will try this once again after 3.5.0.1 gets integrated.

          Show
          Petr Škoda added a comment - lol, thanks a lot! I will try this once again after 3.5.0.1 gets integrated.
          Hide
          Petr Škoda added a comment -

          I think we can get this integrated soon after master is opened for 2.4dev...

          Show
          Petr Škoda added a comment - I think we can get this integrated soon after master is opened for 2.4dev...
          Hide
          Sam Marshall added a comment -

          Fairly comprehensive test script added (and tested).

          I fixed the bugs with the plugins, which were to do with the file references in the PHP files not using the correct URLs. In order to make this easier, I change the plugin API slightly to make a function you can use to get a plugin (from the editor) and one you can use to get a correct file URL within the tinymce folder in a plugin. I changed both affected plugins to use this. They seem to work OK now as per test script, please let me know if there are problems I missed.

          (Code is also rebased on latest master, no conflicts this time.)

          Show
          Sam Marshall added a comment - Fairly comprehensive test script added (and tested). I fixed the bugs with the plugins, which were to do with the file references in the PHP files not using the correct URLs. In order to make this easier, I change the plugin API slightly to make a function you can use to get a plugin (from the editor) and one you can use to get a correct file URL within the tinymce folder in a plugin. I changed both affected plugins to use this. They seem to work OK now as per test script, please let me know if there are problems I missed. (Code is also rebased on latest master, no conflicts this time.)
          Hide
          Sam Marshall added a comment -

          (Just minor tweaks to test script, no practical change.)

          Show
          Sam Marshall added a comment - (Just minor tweaks to test script, no practical change.)
          Hide
          Sam Marshall added a comment -

          Changing to indicate this is for 2.4 (now that a branch exists).

          I have rebased against latest master, which did include work and risk (I think I got it right though) because of other changes to editor - so please let's consider this soon if possible.

          Show
          Sam Marshall added a comment - Changing to indicate this is for 2.4 (now that a branch exists). I have rebased against latest master, which did include work and risk (I think I got it right though) because of other changes to editor - so please let's consider this soon if possible.
          Hide
          Petr Škoda added a comment -

          My +10 to get this in master as soon as it opened for new development, I am going to review it properly once we get there and comment here.

          Show
          Petr Škoda added a comment - My +10 to get this in master as soon as it opened for new development, I am going to review it properly once we get there and comment here.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          +100/100 (I'm shy to vote more than +1, lol)

          Show
          Eloy Lafuente (stronk7) added a comment - +100/100 (I'm shy to vote more than +1, lol)
          Hide
          Petr Škoda added a comment -

          Sam: could you please rebase this on top of current master? And please move also the spell plugin which is not part of upstream tinymce. I am not planning any new code drop from upstream until we resolve this. Thanks in advance.

          Show
          Petr Škoda added a comment - Sam: could you please rebase this on top of current master? And please move also the spell plugin which is not part of upstream tinymce. I am not planning any new code drop from upstream until we resolve this. Thanks in advance.
          Hide
          Michael Aherne added a comment -

          I tested this by packaging the Universitaet Tuebingen's excellent Cloze format editor as a plugin, and can't find any problems with it. The only thing that's not ideal is having to use the '/../../../pluginname/dialog.php' format for PHP files, but I can't see how that could be implemented better, and it's really not that much of a problem anyway!

          Show
          Michael Aherne added a comment - I tested this by packaging the Universitaet Tuebingen's excellent Cloze format editor as a plugin, and can't find any problems with it. The only thing that's not ideal is having to use the '/../../../pluginname/dialog.php' format for PHP files, but I can't see how that could be implemented better, and it's really not that much of a problem anyway!
          Hide
          Petr Škoda added a comment -

          hi, the relative links with ../../.. will not work once we implement some optional loader, all paths should be using paths relative to editor root or wwwroot

          Show
          Petr Škoda added a comment - hi, the relative links with ../../.. will not work once we implement some optional loader, all paths should be using paths relative to editor root or wwwroot
          Hide
          Michael Aherne added a comment -

          Hi Petr, when I used relative paths for PHP files they were being served by the loader as text rather than executed as PHP. I was assuming that the ../../.. was being used to bypass the loader altogether?

          Show
          Michael Aherne added a comment - Hi Petr, when I used relative paths for PHP files they were being served by the loader as text rather than executed as PHP. I was assuming that the ../../.. was being used to bypass the loader altogether?
          Hide
          Petr Škoda added a comment -

          There will be probably some JS variable that contains the the tinymce custom plugins folder - such as ed.getParam("pluginsrootphp") + "/yourpluginname/dialog.php".

          Hmm, the loader relative url may actually work, but I do not think we should rely on it because it might break if we implement different loader/caching techniques in the future.

          Show
          Petr Škoda added a comment - There will be probably some JS variable that contains the the tinymce custom plugins folder - such as ed.getParam("pluginsrootphp") + "/yourpluginname/dialog.php". Hmm, the loader relative url may actually work, but I do not think we should rely on it because it might break if we implement different loader/caching techniques in the future.
          Hide
          Sam Marshall added a comment -

          Note: I have rebased to current master but have not yet moved the spellchecker plugin (am halfway through that but got called away), or tested it for that matter.

          Show
          Sam Marshall added a comment - Note: I have rebased to current master but have not yet moved the spellchecker plugin (am halfway through that but got called away), or tested it for that matter.
          Hide
          Nadav Kavalerchik added a comment -

          Waiting for this to be integrated into Moodle 2.4dev master branch, so I could start converting my (lovely) CONTRIB-2730 set of plugins I made for HTMLAREA on Moodle 19.

          Show
          Nadav Kavalerchik added a comment - Waiting for this to be integrated into Moodle 2.4dev master branch, so I could start converting my (lovely) CONTRIB-2730 set of plugins I made for HTMLAREA on Moodle 19.
          Hide
          Sam Marshall added a comment -

          I have updated this change a bit to fix minor errors, and added a new commit to move the spellchecker into the plugins folder. The testing instructions have been updated to include the spellchecker.

          Petr is going to take this over (for a bit anyway) in order to do work on ensuring that the process of updating TinyMCE from upstream is simple/minimal.

          Show
          Sam Marshall added a comment - I have updated this change a bit to fix minor errors, and added a new commit to move the spellchecker into the plugins folder. The testing instructions have been updated to include the spellchecker. Petr is going to take this over (for a bit anyway) in order to do work on ensuring that the process of updating TinyMCE from upstream is simple/minimal.
          Hide
          Petr Škoda added a comment -

          Hello Sam,

          I have rebased your https://github.com/sammarshallou/moodle/compare/master...MDL-33041-master and added new TinMCE 3.6.0 and cleanup commits on top of your work (without any significant changes).

          I believe it is now suitable for integration, if there are any problems or ideas for more improvements we can address them promptly next week.

          My changes:

          • new TinyMCE 3.6.0 with only one modification - string loading (yay!)
          • forked advimage to separate plugin, the changes in UI were too big imo to maintain it as patches
          • reworked plugin strings - they need to be in standard "pluginname:stringname" format to maintain consistency with standard TinyMCE plugins
          • improved phpdocs (I know you do not like the indentation in docs blocks, but that is the way most of the code does it now)
          • reworked moodlemedia preview
          • added new JS variable for our plugin root - I hate those /../../../../..
          • added new PHP method for tiny_mce JS code $editor->get_tinymce_base_url() that matches tinyMCE.baseURL available in JS - again I hate counting ../../../../..

          I suppose now is the time for plugin developers to start converting current code and report any problems or suggest improvements.

          Thanks a lot everybody!

          Show
          Petr Škoda added a comment - Hello Sam, I have rebased your https://github.com/sammarshallou/moodle/compare/master...MDL-33041-master and added new TinMCE 3.6.0 and cleanup commits on top of your work (without any significant changes). I believe it is now suitable for integration, if there are any problems or ideas for more improvements we can address them promptly next week. My changes: new TinyMCE 3.6.0 with only one modification - string loading (yay!) forked advimage to separate plugin, the changes in UI were too big imo to maintain it as patches reworked plugin strings - they need to be in standard "pluginname:stringname" format to maintain consistency with standard TinyMCE plugins improved phpdocs (I know you do not like the indentation in docs blocks, but that is the way most of the code does it now) reworked moodlemedia preview added new JS variable for our plugin root - I hate those /../../../../.. added new PHP method for tiny_mce JS code $editor->get_tinymce_base_url() that matches tinyMCE.baseURL available in JS - again I hate counting ../../../../.. I suppose now is the time for plugin developers to start converting current code and report any problems or suggest improvements. Thanks a lot everybody!
          Hide
          Dan Poltawski added a comment -

          I've added the api_change and dev_docs_required to this issue. Maybe if/when this is integrated we could 'announce it' on the moodle.org forums. I'm sure there are many interested parties who'd like to try out the new capabilities.

          Show
          Dan Poltawski added a comment - I've added the api_change and dev_docs_required to this issue. Maybe if/when this is integrated we could 'announce it' on the moodle.org forums. I'm sure there are many interested parties who'd like to try out the new capabilities.
          Hide
          Dan Poltawski added a comment -

          Hi Everyone,

          This is looking great for integration this week. Here are my comments from review:

          plugins/moodlemedia/tinymce/js/media.js

          • We have changed the way that the media previews are generated here, i'm intruiged what cause
            the change to add encode64 and can we test explicitly this change? Seems unrelated to these changes
            in general.

          plugins/loader.php

          • Can one of you verify my assumption that get_file_argument() gets rid of all the nasites
            and the tail end of that regeexp is rock solid from ../ etc.
          • I think the first of these comments is misleading (reffering to 'dev' which has been replaced
            with -1, at a guess):
            // Note that version number is totally ignored, user can specify anything,
            // except for the difference between 'dev' and anything else.
            
            // We don't actually care what the version number is but there is a special
            // case for '-1' which means, set the files to not be cached.
            

          upgrade.txt

          • I think this plugins support is worthy of an upgrade.txt mention? Maybe in lib/?

          Whitespace

          • The following files have whitespace problems (tabs mostly I think). As far as I know they are all under
            moodle control so we shouldn't have tabs in them:
            b/lib/editor/tinymce/plugins/dragmath/dragmath.php
            b/lib/editor/tinymce/plugins/dragmath/tinymce/editor_plugin.js
            b/lib/editor/tinymce/plugins/moodleimage/tinymce/editor_plugin.js
            b/lib/editor/tinymce/plugins/moodleimage/tinymce/image.htm
            b/lib/editor/tinymce/plugins/moodleimage/tinymce/js/image.js
            b/lib/editor/tinymce/plugins/moodlemedia/tinymce/editor_plugin.js
            b/lib/editor/tinymce/plugins/moodlemedia/tinymce/moodlemedia.htm
            b/lib/editor/tinymce/plugins/moodlenolink/tinymce/editor_plugin.js
          Show
          Dan Poltawski added a comment - Hi Everyone, This is looking great for integration this week. Here are my comments from review: plugins/moodlemedia/tinymce/js/media.js We have changed the way that the media previews are generated here, i'm intruiged what cause the change to add encode64 and can we test explicitly this change? Seems unrelated to these changes in general. plugins/loader.php Can one of you verify my assumption that get_file_argument() gets rid of all the nasites and the tail end of that regeexp is rock solid from ../ etc. I think the first of these comments is misleading (reffering to 'dev' which has been replaced with -1, at a guess): // Note that version number is totally ignored, user can specify anything, // except for the difference between 'dev' and anything else . // We don't actually care what the version number is but there is a special // case for '-1' which means, set the files to not be cached. upgrade.txt I think this plugins support is worthy of an upgrade.txt mention? Maybe in lib/? Whitespace The following files have whitespace problems (tabs mostly I think). As far as I know they are all under moodle control so we shouldn't have tabs in them: b/lib/editor/tinymce/plugins/dragmath/dragmath.php b/lib/editor/tinymce/plugins/dragmath/tinymce/editor_plugin.js b/lib/editor/tinymce/plugins/moodleimage/tinymce/editor_plugin.js b/lib/editor/tinymce/plugins/moodleimage/tinymce/image.htm b/lib/editor/tinymce/plugins/moodleimage/tinymce/js/image.js b/lib/editor/tinymce/plugins/moodlemedia/tinymce/editor_plugin.js b/lib/editor/tinymce/plugins/moodlemedia/tinymce/moodlemedia.htm b/lib/editor/tinymce/plugins/moodlenolink/tinymce/editor_plugin.js
          Hide
          Petr Škoda added a comment - - edited

          1/ let me repeat it once again - you can not pass urls around in GET parameter because there are "security" filters that remove it

          2/ yes, the get_file_argument() removes all nasties - I was tricked by that too and tried to hack around it, I am going to add comment there to explain it

          3/ fixing the 'dev'

          4/ I thought it is enough to announce this new subplugin type in release notes, there was no support for custom plugins before in tinymce.

          5/ moodleimage contains tabs intentionally because it is a fork of advimage that will have to be kept in sync - we always tried to make as few changes there as possible; the same for spellchecker plugin

          6/ I will fix dragmath and nolink plugins now

          Show
          Petr Škoda added a comment - - edited 1/ let me repeat it once again - you can not pass urls around in GET parameter because there are "security" filters that remove it 2/ yes, the get_file_argument() removes all nasties - I was tricked by that too and tried to hack around it, I am going to add comment there to explain it 3/ fixing the 'dev' 4/ I thought it is enough to announce this new subplugin type in release notes, there was no support for custom plugins before in tinymce. 5/ moodleimage contains tabs intentionally because it is a fork of advimage that will have to be kept in sync - we always tried to make as few changes there as possible; the same for spellchecker plugin 6/ I will fix dragmath and nolink plugins now
          Hide
          Tim Hunt added a comment -

          1/ You can pass around the bit of the URL that comes after wwwroot (e.g. mod/quiz/view.php?id=123). Quiz and question bank does that in some places. We even have PARAM_LOCALURL to validate it.

          Show
          Tim Hunt added a comment - 1/ You can pass around the bit of the URL that comes after wwwroot (e.g. mod/quiz/view.php?id=123). Quiz and question bank does that in some places. We even have PARAM_LOCALURL to validate it.
          Hide
          Petr Škoda added a comment -

          1/ but in case of moodlemedia plugin the URL may be in any format - youtube, vimeo, draftfile, whatever, so PARAM_LOCALURL will not work there

          Show
          Petr Škoda added a comment - 1/ but in case of moodlemedia plugin the URL may be in any format - youtube, vimeo, draftfile, whatever, so PARAM_LOCALURL will not work there
          Hide
          Petr Škoda added a comment -

          Two commits added, thanks a lot for the review.

          Show
          Petr Škoda added a comment - Two commits added, thanks a lot for the review.
          Hide
          Nadav Kavalerchik added a comment -

          How about adding the AutoSave plugin to core?
          http://www.tinymce.com/wiki.php/API3:class.tinymce.plugins.AutoSave
          ( I am using it and it is very useful )

          Also, the ImgMAP plugin is very popular with our teachers too (http://code.google.com/p/imgmap/)

          Show
          Nadav Kavalerchik added a comment - How about adding the AutoSave plugin to core? http://www.tinymce.com/wiki.php/API3:class.tinymce.plugins.AutoSave ( I am using it and it is very useful ) Also, the ImgMAP plugin is very popular with our teachers too ( http://code.google.com/p/imgmap/ )
          Hide
          Petr Škoda added a comment -

          Hi, autosave is discussed elsewhere - in short the problem is there is no place to autosave it in moodle, sorry. In any case this is not the place to discuss introduction of new standard plugins, this issue deals only with refactoring, there will be new issues for more improvements such as custom editor buttons or editor subplugin settings.

          Show
          Petr Škoda added a comment - Hi, autosave is discussed elsewhere - in short the problem is there is no place to autosave it in moodle, sorry. In any case this is not the place to discuss introduction of new standard plugins, this issue deals only with refactoring, there will be new issues for more improvements such as custom editor buttons or editor subplugin settings.
          Hide
          Dan Poltawski added a comment -

          Thanks. I've integrated this now.

          Show
          Dan Poltawski added a comment - Thanks. I've integrated this now.
          Hide
          Dan Poltawski added a comment -

          Hmm, I have just realised that I think this is completely missing from the admin UI? Not even in the plugins overview page as far as I can see.

          Show
          Dan Poltawski added a comment - Hmm, I have just realised that I think this is completely missing from the admin UI? Not even in the plugins overview page as far as I can see.
          Hide
          Petr Škoda added a comment -

          It should be at the end of the plugins overview, I am going to add uninstall support next week, but it will require some core tweaks I did not want to do as part of this issue, thanks.

          Show
          Petr Škoda added a comment - It should be at the end of the plugins overview, I am going to add uninstall support next week, but it will require some core tweaks I did not want to do as part of this issue, thanks.
          Hide
          Jean-Michel Vedrine added a comment -

          Hi Dan,
          I do see TinyMCE plugins in the plugins overview (see attached screenshot). As you see I lost no time in converting some plugins I use
          Maybe later it would be good that the Availability and Actions columns can be used too for these plugins. But this can wait, it's best to first integrate the present issue, other features can wait IMHO.

          Show
          Jean-Michel Vedrine added a comment - Hi Dan, I do see TinyMCE plugins in the plugins overview (see attached screenshot). As you see I lost no time in converting some plugins I use Maybe later it would be good that the Availability and Actions columns can be used too for these plugins. But this can wait, it's best to first integrate the present issue, other features can wait IMHO.
          Hide
          Dan Poltawski added a comment -

          Yep, sorry. I think I must've been searching for 'dragmaths'.

          Show
          Dan Poltawski added a comment - Yep, sorry. I think I must've been searching for 'dragmaths'.
          Hide
          Petr Škoda added a comment -

          I have created new META for the rest of issues MDL-34875.

          Show
          Petr Škoda added a comment - I have created new META for the rest of issues MDL-34875 .
          Hide
          Tim Barker added a comment -

          An epic test but it passed!

          Show
          Tim Barker added a comment - An epic test but it passed!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility!

          Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility! Many thanks for your collaboration, yay! Closing, ciao
          Hide
          Nadav Kavalerchik added a comment -

          Definitely for the good! thank you all, very much

          Show
          Nadav Kavalerchik added a comment - Definitely for the good ! thank you all, very much

            People

            • Votes:
              12 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: