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
    • Rank:
      17581

      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.

        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 Škoda added a comment -

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

        Show
        Petr Škoda 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: