Moodle
  1. Moodle
  2. MDL-34050

Matching Question in lesson does not recognise HTML format in answer text

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.3, 2.3.4, 2.4.1, 2.5
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Lesson
    • Environment:
      Moodle Version 2.2.3 (20120615)
    • Testing Instructions:
      Hide
      1. Create a lesson
      2. Add a Question page of type 'Matching'
      3. Fill in a Title; Description text; correct answer response; wrong answer response;
      4. Add question text one "Blue is a..."- SELECT HTML FORMAT; Matching Answer Text one "Colour".
      5. Add question text two "Cold is a..."- SELECT Moodle FORMAT; Matching Answer Text two "Temperature".
      6. Save the question
      7. Open the question in 'Preview' mode - apply the correct matches to the correct answer (Blue is a... COLOUR || Cold is a... Temperature)
      8. Submit the answer

      It should returns the 'Correct Answer' response.

      Show
      Create a lesson Add a Question page of type 'Matching' Fill in a Title; Description text; correct answer response; wrong answer response; Add question text one "Blue is a..."- SELECT HTML FORMAT; Matching Answer Text one "Colour". Add question text two "Cold is a..."- SELECT Moodle FORMAT; Matching Answer Text two "Temperature". Save the question Open the question in 'Preview' mode - apply the correct matches to the correct answer (Blue is a... COLOUR || Cold is a... Temperature) Submit the answer It should returns the 'Correct Answer' response.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-34050_m24
    • Pull Master Branch:
    • Rank:
      42352

      Description

      I have created a lesson that contains Matching Questions. When I created the different matching options I discovered that if I used the HTML option for both Column A and Column B (Question||Correct Answer) that the answer would not be recognised, and it would be impossible to get the application to acknowldge a correct match. As I explored further I discovered that if I used the HTML on Column A only (Question), and left Column B as MoodleAutoFormat (Answer) then I could have colours in my question text, and the application would recognise the correct answers.

      I also tested this in the current demonstration site at WWW.Moodle.org to ensure that it wasn't just our installation - and got the same result.

      To replicate:

      1. Create a lesson
      2. Add a Question page of type 'Matching'
      3. Fill in a Title; Description text; correct answer response; wrong answer response; Question text one "Blue is a..."- SELECT HTML FORMAT; Matching Answer Text one "Colour"- SELECT HTML FORMAT: Question text two "Cold is a..." - SELECT HTML FORMAT; Matching Answer Text two "Temperature" - SELECT HTML FORMAT
      4. Save the question
      5. Re-Open the question in Edit mode, the tiny MCE editors are now available for the question/matching text....
      6. Apply HTML formating to Question text one and Question text two...
      7. no need to add any to the matching text, as they appear in a drop down and do not get formatted...
      8. Save the question
      9. Open the question in 'Preview' mode - apply the correct matches to the correct answer (Blue is a... COLOUR || Cold is a... Temperature)
      10. Submit the answer and the application returns the 'Wrong Answer' response.
      11. Create a second question, replicating the first, however in the second question only SELECT HTML FORMAT for the question text, and leave the Matching Answer text answers as the MOODLE AUTO FORMAT -
      12. Save and re-open the question editor to apply the formating as above...
      13. Save the changes to the 2nd question...
      14. Now open in the Preview mode - and answer the questions - the correct responses will now give the correct answer response.

      If this is not a bug - can someone move this to a product suggestion, can the Moodle Docs be updated to inform users about this limitation? - I mistakenly believed that the format of the Answer should match the question... but that does not appear to be the case... Once SELECT HTML FORMAT has been added to a question/answer it is impossible to turn it off, so the only work around is to re-create the question...and scrap the original

        Issue Links

          Activity

          Hide
          Steven Evans added a comment -

          These are two screen shots with the different formats applied to the answer text and the result display for each

          Show
          Steven Evans added a comment - These are two screen shots with the different formats applied to the answer text and the result display for each
          Hide
          Shane Elliott added a comment -

          We have also received reports of this issue from some of our clients. Ideally the question type should work regardless of the format chosen.

          Show
          Shane Elliott added a comment - We have also received reports of this issue from some of our clients. Ideally the question type should work regardless of the format chosen.
          Hide
          Michael de Raadt added a comment -

          This issue appears to have been reported a number of times. I will try to tidy the web of issues included.

          A patch has been provided by Joseph Rézeau in MDL-35448.

          I'm going to leave this issue open as it appears to have the most detail and partner backing.

          Show
          Michael de Raadt added a comment - This issue appears to have been reported a number of times. I will try to tidy the web of issues included. A patch has been provided by Joseph Rézeau in MDL-35448 . I'm going to leave this issue open as it appears to have the most detail and partner backing.
          Hide
          Joseph Rézeau added a comment -

          @Michael,
          The real problem is quite simple. When creating in Lesson a question of any type, the Answer and Response field have a "format" dropdown box with a choice of 4 formats. This is a leftover from much older versions of Moodle, and does not make any sense in Moodle 2!

          The solution is to simply REMOVE once for all that useless and misleading dropdown box.

          For all question types except matching, provide HTML editor for Question and Answer fields.
          For the matching question type, provide HTML editor for Question field and plain text editor for Answer field.
          That is to say: provide the same question editing interface as for Quiz.

          Show
          Joseph Rézeau added a comment - @Michael, The real problem is quite simple. When creating in Lesson a question of any type, the Answer and Response field have a "format" dropdown box with a choice of 4 formats. This is a leftover from much older versions of Moodle, and does not make any sense in Moodle 2! The solution is to simply REMOVE once for all that useless and misleading dropdown box. For all question types except matching , provide HTML editor for Question and Answer fields. For the matching question type, provide HTML editor for Question field and plain text editor for Answer field. That is to say: provide the same question editing interface as for Quiz.
          Hide
          Rossiani Wijaya added a comment -

          Hi Joseph,

          Thank you for your suggestion to fix this issue.

          I think it would be better to fix the comparison answer by removing tag from it.

          Please take a look the patch and provide some feedback.

          I will backport the patch after it pass peer-review.

          Sending for peer review.

          Show
          Rossiani Wijaya added a comment - Hi Joseph, Thank you for your suggestion to fix this issue. I think it would be better to fix the comparison answer by removing tag from it. Please take a look the patch and provide some feedback. I will backport the patch after it pass peer-review. Sending for peer review.
          Hide
          Ankit Agarwal added a comment -

          Hi Rosie,
          This looks good
          [Y] Syntax
          [?] Output The output contains HTML which I am not sure is expected behaviour. But clearly that is a separate issue.
          [y] Whitespace
          [Y] Language
          [na] Databases
          [y] Testing
          [-] Security
          [na] Documentation
          [Y] Git
          [Y] Sanity check

          thanks

          Show
          Ankit Agarwal added a comment - Hi Rosie, This looks good [Y] Syntax [?] Output The output contains HTML which I am not sure is expected behaviour. But clearly that is a separate issue. [y] Whitespace [Y] Language [na] Databases [y] Testing [-] Security [na] Documentation [Y] Git [Y] Sanity check thanks
          Hide
          Rossiani Wijaya added a comment -

          Thanks Ankit for reviewing.

          I also noticed the HTML tag on continue page. I created an issue to fix that (MDL-36343).

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Ankit for reviewing. I also noticed the HTML tag on continue page. I created an issue to fix that ( MDL-36343 ). Submitting for integration review.
          Hide
          Joseph Rézeau added a comment -

          @Ankit and Rossiani,
          I have applied your patch to my moodle 2.3 test site and it does not fix the problem at all.
          I maintain that the only reasonable solution is the one I suggested in my post dated 09/Oct/12 5:45 PM:

          For all question types except matching, provide HTML editor for Question and Answer fields.
          For the matching question type, provide HTML editor for Question field and plain text editor for Answer field.
          That is to say: provide the same question editing interface as for Quiz.

          And I do not understand why you opened a new issue report for the current problem. This should really be quite simple to fix and is getting inordinately confused.

          Joseph

          Show
          Joseph Rézeau added a comment - @Ankit and Rossiani, I have applied your patch to my moodle 2.3 test site and it does not fix the problem at all. I maintain that the only reasonable solution is the one I suggested in my post dated 09/Oct/12 5:45 PM: For all question types except matching, provide HTML editor for Question and Answer fields. For the matching question type, provide HTML editor for Question field and plain text editor for Answer field. That is to say: provide the same question editing interface as for Quiz . And I do not understand why you opened a new issue report for the current problem. This should really be quite simple to fix and is getting inordinately confused. Joseph
          Hide
          Rossiani Wijaya added a comment -

          Hi Joseph,

          Thank you for testing the patch.

          I just re-tested the patch again and it works fine.

          The new issue is created to fix the display. I posted a screenshot on MDL-36343.

          Show
          Rossiani Wijaya added a comment - Hi Joseph, Thank you for testing the patch. I just re-tested the patch again and it works fine. The new issue is created to fix the display. I posted a screenshot on MDL-36343 .
          Hide
          Joseph Rézeau added a comment -

          Hi Rossiani,

          Sorry for my misunderstanding.

          Your patch does correct one part of the bug, i.e. it does match correctly the right column items to the left column items even if those right column items were created with HTML tags.

          But in my previous comment I wrote that it does not solve the whole issue, because the HTML tags remain visible on the Continue page, as you rightly remarked.

          So my conclusion remains the same: we must attack the source of the problem, not its consequences. That is to say we must absolutely provide a matching questions creation interface with no access to the HTML editor in the "Answer" (aka right column) items. If we continue to provide the possibility to format those "Ansswer" items, it gives the teacher the false impression that it is possible to do so, whereas, since Answers are displayed in a dropdown box, it is quite impossible to have formatted items there.

          Again, I maintain that the best (and only) solution is to provide (for the Lesson Matching Questions ) the same interface as for the Quiz module Matching Questions creation.

          I will try to provide a relevant patch as soon as possible this week.

          Joseph

          Show
          Joseph Rézeau added a comment - Hi Rossiani, Sorry for my misunderstanding. Your patch does correct one part of the bug, i.e. it does match correctly the right column items to the left column items even if those right column items were created with HTML tags. But in my previous comment I wrote that it does not solve the whole issue, because the HTML tags remain visible on the Continue page, as you rightly remarked. So my conclusion remains the same: we must attack the source of the problem, not its consequences. That is to say we must absolutely provide a matching questions creation interface with no access to the HTML editor in the "Answer" (aka right column) items. If we continue to provide the possibility to format those "Ansswer" items, it gives the teacher the false impression that it is possible to do so, whereas, since Answers are displayed in a dropdown box, it is quite impossible to have formatted items there. Again, I maintain that the best (and only) solution is to provide (for the Lesson Matching Questions ) the same interface as for the Quiz module Matching Questions creation. I will try to provide a relevant patch as soon as possible this week. Joseph
          Hide
          Joseph Rézeau added a comment -

          Here is my patch (against 2.3) which removes the HTML editor from the matching question "Matches with answer" field.

          https://github.com/rezeau/moodle/compare/MOODLE_23_STABLE...MDL-34050_m23

          Show
          Joseph Rézeau added a comment - Here is my patch (against 2.3) which removes the HTML editor from the matching question "Matches with answer" field. https://github.com/rezeau/moodle/compare/MOODLE_23_STABLE...MDL-34050_m23
          Hide
          Dan Poltawski added a comment -

          Hi Rosie,

          It seems to me that Joseph's solution to this problem is really the right solution. We shouldn't be allowing HTML at all? Is there any reason why we should do so? If so, then I don't think this fix is enough.

          With regard to this suggested fix, htmlspecialchars_decode is not the 'Moodle' way to do this, we have format_string for that in Moodle, which handles utf8 properly too.

          Show
          Dan Poltawski added a comment - Hi Rosie, It seems to me that Joseph's solution to this problem is really the right solution. We shouldn't be allowing HTML at all? Is there any reason why we should do so? If so, then I don't think this fix is enough. With regard to this suggested fix, htmlspecialchars_decode is not the 'Moodle' way to do this, we have format_string for that in Moodle, which handles utf8 properly too.
          Hide
          Joseph Rézeau added a comment -

          Dan "It seems to me that Joseph's solution to this problem is really the right solution."
          Yes it is.

          Dan "We shouldn't be allowing HTML at all? Is there any reason why we should do so?"
          No, again and once more, we MUST NOT allow HTML in the Answer field of the matching questions, simply because the contents of that field will be displayed in a drop-down list, where it is impossible to display HTML. It's as simple as that.

          Show
          Joseph Rézeau added a comment - Dan "It seems to me that Joseph's solution to this problem is really the right solution." Yes it is. Dan "We shouldn't be allowing HTML at all? Is there any reason why we should do so?" No, again and once more, we MUST NOT allow HTML in the Answer field of the matching questions, simply because the contents of that field will be displayed in a drop-down list, where it is impossible to display HTML. It's as simple as that.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Rossiani Wijaya added a comment -

          Hi Joseph,

          Thank you for providing the patch.

          I agree with you that the answer shouldn't contained any HTML tag. The patch works great to limit the the use of html editor for the answer. However, beside fixing the interface itself, I think the code also needs to be modified to remove any HTML tag before inserting it to DB.

          I'm planning to do the above I mentioned plus updating the existing answer in DB. Any feeback would be appreciated.

          Thanks.

          Show
          Rossiani Wijaya added a comment - Hi Joseph, Thank you for providing the patch. I agree with you that the answer shouldn't contained any HTML tag. The patch works great to limit the the use of html editor for the answer. However, beside fixing the interface itself, I think the code also needs to be modified to remove any HTML tag before inserting it to DB. I'm planning to do the above I mentioned plus updating the existing answer in DB. Any feeback would be appreciated. Thanks.
          Hide
          Rossiani Wijaya added a comment -

          As I'm working on the patch, I think it is better to improve the functionality for checking user's answer rather than stripping off the tag.

          Joseph, I added some changes on top of your patch. Feel free to leave any comment for the changes.

          Rosie

          Show
          Rossiani Wijaya added a comment - As I'm working on the patch, I think it is better to improve the functionality for checking user's answer rather than stripping off the tag. Joseph, I added some changes on top of your patch. Feel free to leave any comment for the changes. Rosie
          Hide
          Ankit Agarwal added a comment -

          Just noting had a discussion with Rosie, and we decided it is better to support indexing by id in get_answers() api rather than attempting it afterwords.
          Thanks

          Show
          Ankit Agarwal added a comment - Just noting had a discussion with Rosie, and we decided it is better to support indexing by id in get_answers() api rather than attempting it afterwords. Thanks
          Hide
          Rossiani Wijaya added a comment -

          Dear Integrators,

          Could you please provide some feedback regarding these patches and help me decide on which patch is better for integration.

          I'm fixing the logic of the code so the answer is compared by id, instead of their value.

          Currently, get_answers() will only run if $this->answers is null.

          $this-answers is process when the lesson page is loaded. Therefore there's no way to reprocess the function and force changing the index to id within the function, might increase the possibility of breaking the code in other places.

          Possible solution is to fix get_answers() by adding two new params to set the answer id as array key and re-process the value of $this->answers. (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050_api). I have to admit it seems like a hack.

          The other option is to keep the current get_answers() and re-process the answers array ids within the code (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050)

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Dear Integrators, Could you please provide some feedback regarding these patches and help me decide on which patch is better for integration. I'm fixing the logic of the code so the answer is compared by id, instead of their value. Currently, get_answers() will only run if $this->answers is null. $this-answers is process when the lesson page is loaded. Therefore there's no way to reprocess the function and force changing the index to id within the function, might increase the possibility of breaking the code in other places. Possible solution is to fix get_answers() by adding two new params to set the answer id as array key and re-process the value of $this->answers. (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050_api ). I have to admit it seems like a hack. The other option is to keep the current get_answers() and re-process the answers array ids within the code (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050 ) Thanks Rosie
          Hide
          Dan Poltawski added a comment -

          Hi Rosie,

          I think I prefer the second option from what i've read:

          The other option is to keep the current get_answers() and re-process the answers array ids within the code (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050)

          Show
          Dan Poltawski added a comment - Hi Rosie, I think I prefer the second option from what i've read: The other option is to keep the current get_answers() and re-process the answers array ids within the code (patch: https://github.com/rwijaya/moodle/compare/master...MDL-34050 )
          Hide
          Rossiani Wijaya added a comment -

          Thank you for the feedback Dan.

          Submitting this for integration review.

          Show
          Rossiani Wijaya added a comment - Thank you for the feedback Dan. Submitting this for integration review.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
          Hide
          Mark Nelson added a comment -

          Works as expected, passing. I did however find an unrelated issue that I will be filing a bug for. Thanks.

          Show
          Mark Nelson added a comment - Works as expected, passing. I did however find an unrelated issue that I will be filing a bug for. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

            People

            • Votes:
              3 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: