Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3, 2.1, 2.2
    • Fix Version/s: 2.1.1
    • Component/s: HTML Editor (TinyMCE)
    • Labels:
    • Testing Instructions:
      Hide

      1. Create a new multiple-choice question (or edit an existing one)
      2. Verify that the Choice and Choice feedback HTML editors are small - just big enough for about one line of text.
      3. Verify that any HTML editor can be resized from to minimum size or any larger size.

      Note that it you web browser may not notice that the TinyMCE JavaScript has changed. You may need to do a forced reload (E.g. SHIFT + F5 in firefox) for it to work.

      Show
      1. Create a new multiple-choice question (or edit an existing one) 2. Verify that the Choice and Choice feedback HTML editors are small - just big enough for about one line of text. 3. Verify that any HTML editor can be resized from to minimum size or any larger size. Note that it you web browser may not notice that the TinyMCE JavaScript has changed. You may need to do a forced reload (E.g. SHIFT + F5 in firefox) for it to work.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE

      Description

      There is a minimum size of 100px for TinyMCE, which is unnecessarily big.

      Fixing this comes in two parts. It requires a change to TinyMCE to get rid of some unnecesary hard-coding of the 100px. See:
      http://tinymce.moxiecode.com/develop/bugtracker_view.php?id=3789
      https://github.com/tinymce/tinymce/pull/69

      Then we need to change our configuration, which I am about to create a commit for.

        Gliffy Diagrams

          Activity

          Hide
          Tim Hunt added a comment -

          OK, this is absolutely not ready for integration yet, because it involves a change to TinyMCE core. See above. However, I would appreciate any review comments.

          Show
          Tim Hunt added a comment - OK, this is absolutely not ready for integration yet, because it involves a change to TinyMCE core. See above. However, I would appreciate any review comments.
          Hide
          Michael de Raadt added a comment -

          Apu, could you have a look at this code for Tim.

          Show
          Michael de Raadt added a comment - Apu, could you have a look at this code for Tim.
          Hide
          Tim Hunt added a comment -

          Editing Multiple choice questions in Moodle 2.1 will really suck if we don't do this. Please may I have some feedback.

          Show
          Tim Hunt added a comment - Editing Multiple choice questions in Moodle 2.1 will really suck if we don't do this. Please may I have some feedback.
          Hide
          Tim Hunt added a comment -

          Just a note that since we changed to TinyMCE 3.4.2 in master, this needs some more work before it is integrated.

          Show
          Tim Hunt added a comment - Just a note that since we changed to TinyMCE 3.4.2 in master, this needs some more work before it is integrated.
          Hide
          Aparup Banerjee added a comment -

          Hi, sorry i missed this totally, will look at this very soon.

          Show
          Aparup Banerjee added a comment - Hi, sorry i missed this totally, will look at this very soon.
          Hide
          Aparup Banerjee added a comment -

          Hi Tim, this code looks fine to me. testing it seems fine too.

          Show
          Aparup Banerjee added a comment - Hi Tim, this code looks fine to me. testing it seems fine too.
          Hide
          Tim Hunt added a comment -

          OK, so what is the procedure for getting this into Moodle? Do I need to fork https://github.com/moodle/custom-tinymce and prepare some commits against that?

          I wonder if the Moxiecode people will ever respond to the pull request I sent them.

          Show
          Tim Hunt added a comment - OK, so what is the procedure for getting this into Moodle? Do I need to fork https://github.com/moodle/custom-tinymce and prepare some commits against that? I wonder if the Moxiecode people will ever respond to the pull request I sent them.
          Hide
          Aparup Banerjee added a comment -

          yea, it would be great to have Moxiecode respond, afaik your changes would be have to be integrated into custom-tinymce (via github) and moodle (via git.moodle.org) both.

          I'm not sure if those changes would require a rebuild within custom-tinymce, really no idea there but to be safe,
          there's a readme file about rebuilding at: https://github.com/moodle/custom-tinymce/blob/MOODLE_21_STABLE/readme.md

          The readme file also takes you through steps that i went through in MDL-27500 to copy changes over into the moodle.git and hence integration into moodle.

          its an unusual process i think mainly to keep tinymce's building 'materials' out of moodle.

          it would help to ask Petr too.

          Show
          Aparup Banerjee added a comment - yea, it would be great to have Moxiecode respond, afaik your changes would be have to be integrated into custom-tinymce (via github) and moodle (via git.moodle.org) both. I'm not sure if those changes would require a rebuild within custom-tinymce, really no idea there but to be safe, there's a readme file about rebuilding at: https://github.com/moodle/custom-tinymce/blob/MOODLE_21_STABLE/readme.md The readme file also takes you through steps that i went through in MDL-27500 to copy changes over into the moodle.git and hence integration into moodle. its an unusual process i think mainly to keep tinymce's building 'materials' out of moodle. it would help to ask Petr too.
          Show
          Tim Hunt added a comment - There is also https://github.com/moodle/moodle/blob/master/lib/editor/tinymce/readme_moodle.txt
          Hide
          Aparup Banerjee added a comment -

          sorry that's the one i meant.

          Show
          Aparup Banerjee added a comment - sorry that's the one i meant.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Attaching fixed (added parenthesis) and cleaned (from CRLF) patch that will be applied to https://github.com/moodle/custom-tinymce (21_STABLE)

          Show
          Eloy Lafuente (stronk7) added a comment - Attaching fixed (added parenthesis) and cleaned (from CRLF) patch that will be applied to https://github.com/moodle/custom-tinymce (21_STABLE)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          To avoid forgetting it:

          Process to incorporate one local change to tinymce:

          1) apply one patch with the local change against https://github.com/moodle/custom-tinymce (one for each target branch).

          2) rebuild it/them following the steps @ readme_moodle.txt

          3) provide two patches:

          a) one patch with the rebuilt version to be applied to https://github.com/moodle/custom-tinymce (one for each target branch).

          b) another patch with the resulting changes to be applied to moodle.git

          4) Integrate the patches as usual, review/test...

          Done!

          TODO: DOCUMENTE THIS IN THE README_MOODLE !

          Show
          Eloy Lafuente (stronk7) added a comment - To avoid forgetting it: Process to incorporate one local change to tinymce: 1) apply one patch with the local change against https://github.com/moodle/custom-tinymce (one for each target branch). 2) rebuild it/them following the steps @ readme_moodle.txt 3) provide two patches: a) one patch with the rebuilt version to be applied to https://github.com/moodle/custom-tinymce (one for each target branch). b) another patch with the resulting changes to be applied to moodle.git 4) Integrate the patches as usual, review/test... Done! TODO: DOCUMENTE THIS IN THE README_MOODLE !
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Done.

          1) https://github.com/moodle/custom-tinymce/commits/MOODLE_21_STABLE now includes Tim's commit fixed and one more containing some changes that were applied only to moodle.git (no further action needed, everything has been pushed there already).

          2) https://github.com/stronk7/moodle/compare/MOODLE_21_STABLE...MDL-27890_m21 changes to moodle.git (21_STABLE)

          3) https://github.com/stronk7/moodle/compare/master...MDL-27890_master changes to moodle.git (master)

          Going to integrate 2 & 3 now, heading for testing by anyone else...

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Done. 1) https://github.com/moodle/custom-tinymce/commits/MOODLE_21_STABLE now includes Tim's commit fixed and one more containing some changes that were applied only to moodle.git (no further action needed, everything has been pushed there already). 2) https://github.com/stronk7/moodle/compare/MOODLE_21_STABLE...MDL-27890_m21 changes to moodle.git (21_STABLE) 3) https://github.com/stronk7/moodle/compare/master...MDL-27890_master changes to moodle.git (master) Going to integrate 2 & 3 now, heading for testing by anyone else...
          Hide
          Tim Hunt added a comment -

          Thanks for doing this Eloy. Those commits look good to me.

          Show
          Tim Hunt added a comment - Thanks for doing this Eloy. Those commits look good to me.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Wait!

          This will be integrated next week, sorry. Didn't realise it was in peer-review status. In the mean time feel free to use it, will be great for testing because next week will land straight!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Wait! This will be integrated next week, sorry. Didn't realise it was in peer-review status. In the mean time feel free to use it, will be great for testing because next week will land straight! Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          For integrator:

          A) The custom-tinymce repo was updated last week by me with Tim's modifications. No further action needed there.
          B) The points 2) and 3) above are the ones to integrate and test into integration.git

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - For integrator: A) The custom-tinymce repo was updated last week by me with Tim's modifications. No further action needed there. B) The points 2) and 3) above are the ones to integrate and test into integration.git Ciao
          Hide
          Sam Hemelryk added a comment -

          Yay for easy integration instructions. Hopefully they do accept them upstream as it certainly looks like a positive improvement.

          Show
          Sam Hemelryk added a comment - Yay for easy integration instructions. Hopefully they do accept them upstream as it certainly looks like a positive improvement.
          Hide
          Rajesh Taneja added a comment -

          Choice and choice feedback html editors are not small (one line of text) by default.
          Although, html editor can be resized to one line of text or multiple lines.

          Failing as point 2 in testing instructions is not observed.

          Show
          Rajesh Taneja added a comment - Choice and choice feedback html editors are not small (one line of text) by default. Although, html editor can be resized to one line of text or multiple lines. Failing as point 2 in testing instructions is not observed.
          Hide
          Tim Hunt added a comment -

          Are you sure your browser is using the latest JavaScript? Also, TinyMCE remembers the size of each HTML editor during each browser session. So:

          1. Quit your browser, then run it again.

          2. Do a Shift/Ctrl + reload - or, better still, use firebug to open the TinyMCE JavaScript code, and verify that it is really using the latest version.

          Show
          Tim Hunt added a comment - Are you sure your browser is using the latest JavaScript? Also, TinyMCE remembers the size of each HTML editor during each browser session. So: 1. Quit your browser, then run it again. 2. Do a Shift/Ctrl + reload - or, better still, use firebug to open the TinyMCE JavaScript code, and verify that it is really using the latest version.
          Hide
          Rajesh Taneja added a comment -

          Sorry Tim,

          My mistake I pulled wrong version of TinyMCE
          It is working Great. choice and choice feedback are now one-line

          Can you please reset the status, as it works as expected

          Show
          Rajesh Taneja added a comment - Sorry Tim, My mistake I pulled wrong version of TinyMCE It is working Great. choice and choice feedback are now one-line Can you please reset the status, as it works as expected
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (reset)

          Show
          Eloy Lafuente (stronk7) added a comment - (reset)
          Hide
          Rajesh Taneja added a comment -

          Thanks Tim
          Works as expected, Great work Eloy

          Show
          Rajesh Taneja added a comment - Thanks Tim Works as expected, Great work Eloy
          Hide
          Sam Hemelryk added a comment -

          Congratulations - this fix has just been released in the weeklies.

          Show
          Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.
          Hide
          Alexander Ufimtsev added a comment -

          Hi guys!

          Thanks for the amazing work! Do you think something similar can be implemented for this bug, too? http://tracker.moodle.org/browse/MDL-23646

          Many thanks in advance,
          Alexander

          Show
          Alexander Ufimtsev added a comment - Hi guys! Thanks for the amazing work! Do you think something similar can be implemented for this bug, too? http://tracker.moodle.org/browse/MDL-23646 Many thanks in advance, Alexander
          Hide
          Petr Skoda added a comment -

          resolved upstream by https://github.com/tinymce/tinymce/commit/c2c905845a60866b39f33b886a7093c85b817d15, the setting name is min_height, thanks

          Show
          Petr Skoda added a comment - resolved upstream by https://github.com/tinymce/tinymce/commit/c2c905845a60866b39f33b886a7093c85b817d15 , the setting name is min_height, thanks
          Hide
          Tim Hunt added a comment -

          So, do we need a new tracker issue to undo our Moodle-specific changes from this bug, and use the new TinyMCE setting? If you create such an issue, please add me as watcher.

          Show
          Tim Hunt added a comment - So, do we need a new tracker issue to undo our Moodle-specific changes from this bug, and use the new TinyMCE setting? If you create such an issue, please add me as watcher.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: