Moodle
  1. Moodle
  2. MDL-35858

After doing latest weekly update cloze type question does not show correct answers when hovering

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.5, 2.3.2, 2.4
    • Fix Version/s: 2.2.6, 2.3.3
    • Component/s: Quiz
    • Testing Instructions:
      Hide

      Test pre-requisites

      Prepare a quiz with 2 embedded questions (cloze) using the the examples on http://docs.moodle.org/23/en/question/type/multianswer

      Test steps

      1. As a student, take the quiz and answer randomely
      2. Submit your attempt
      3. Review it and make sure you can see the feedback of each subquestion either directly or on mouse over.
      Show
      Test pre-requisites Prepare a quiz with 2 embedded questions (cloze) using the the examples on http://docs.moodle.org/23/en/question/type/multianswer Test steps As a student, take the quiz and answer randomely Submit your attempt Review it and make sure you can see the feedback of each subquestion either directly or on mouse over.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-35858-master
    • Rank:
      44621

      Description

      Just updated on friday to the latest weekly(Moodle 2.3.2+ (Build: 20121005)) and student cant see feedback after taking a quiz which includes cloze questions.
      When i tried it at demo.moodle.net, there it works, so probably something went wrong very recently..
      Also i noticed that in the latest MOODLE_23_STABLE weekly build were unit tests folders included in quiz module and questions, which i think should not be there.

      1. multianswer-patch.txt
        2 kB
        Tõnis Tartes
      1. cloze_demo_moodle_net.jpg
        75 kB
      2. cloze_from_school_moodle.jpg
        75 kB

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          There were some changes made last week, in https://tracker.moodle.org/browse/MDL-35370, but they were tested quite carefully. Please can you attach a screen-grab that shows the problem.

          Show
          Tim Hunt added a comment - There were some changes made last week, in https://tracker.moodle.org/browse/MDL-35370 , but they were tested quite carefully. Please can you attach a screen-grab that shows the problem.
          Hide
          Tõnis Tartes added a comment -

          Attaching screenshots, from demo.moodle.net and from my school moodle.
          The hovering feedback is not appearing on local moodle.

          Show
          Tõnis Tartes added a comment - Attaching screenshots, from demo.moodle.net and from my school moodle. The hovering feedback is not appearing on local moodle.
          Hide
          Tõnis Tartes added a comment -

          I looked into the code and found out that the problem came form question/type/multianswer/renderer.php

          I also compared revisions at github and the change which caused the problem was here - https://github.com/moodle/moodle/commit/d3187c299a14659bd5ec5ccc3e4a243ba3659583#question/type/multianswer/renderer.php

          If you look the multianswer/renderer.php file changes, you will notice that $feedbackimg and $feedbackpopup are left out from label tag.

          I changed those lines back and the feedback started to work again.

          I made a little patch which fixed this.

          Show
          Tõnis Tartes added a comment - I looked into the code and found out that the problem came form question/type/multianswer/renderer.php I also compared revisions at github and the change which caused the problem was here - https://github.com/moodle/moodle/commit/d3187c299a14659bd5ec5ccc3e4a243ba3659583#question/type/multianswer/renderer.php If you look the multianswer/renderer.php file changes, you will notice that $feedbackimg and $feedbackpopup are left out from label tag. I changed those lines back and the feedback started to work again. I made a little patch which fixed this.
          Hide
          Tim Hunt added a comment -

          Fred, please can you fix this regression you caused and get it in to this week's integration? Thanks.

          The fix will involve editing question/type/multianswer/module.js, but given the changes you made to the HTML structure, it is not entirely clear how to do that, otherwise I would have done the fix.

          Show
          Tim Hunt added a comment - Fred, please can you fix this regression you caused and get it in to this week's integration? Thanks. The fix will involve editing question/type/multianswer/module.js, but given the changes you made to the HTML structure, it is not entirely clear how to do that, otherwise I would have done the fix.
          Hide
          Frédéric Massart added a comment -

          Hi Tim,

          I'd like some more input on your part for that. First thing is that I was trying to use the sample provided in the documentation but it outputs an endless notice (http://docs.moodle.org/23/en/question/type/multianswer#Example_2).

          The second thing is that I am not sure how to efficiently work that out. I thought of creating a wrapping tag (div) around the whole subquestion, but it creates an HTML issue where some more styles are needed, and AFAIK we are not supposed to modify the stables too much. The other thing would be that the alignment of the overlay would not be proper, but I'm a bit scare to edit it as I would not like it to overlap the current display. We could use an inline such as a <span> but would that be HTML compliant (as it may contain block type tags).

          And at last, I just realise that this feedback is not accessible. It should also popup on key focus, not only on mouse focus, and use aria attributes (master only surely).

          I am willing to fix this ASAP but as you know this area better than anyone I think I could really use your wisdom here.

          Thanks!

          Show
          Frédéric Massart added a comment - Hi Tim, I'd like some more input on your part for that. First thing is that I was trying to use the sample provided in the documentation but it outputs an endless notice ( http://docs.moodle.org/23/en/question/type/multianswer#Example_2 ). The second thing is that I am not sure how to efficiently work that out. I thought of creating a wrapping tag (div) around the whole subquestion, but it creates an HTML issue where some more styles are needed, and AFAIK we are not supposed to modify the stables too much. The other thing would be that the alignment of the overlay would not be proper, but I'm a bit scare to edit it as I would not like it to overlap the current display. We could use an inline such as a <span> but would that be HTML compliant (as it may contain block type tags). And at last, I just realise that this feedback is not accessible. It should also popup on key focus, not only on mouse focus, and use aria attributes (master only surely). I am willing to fix this ASAP but as you know this area better than anyone I think I could really use your wisdom here. Thanks!
          Hide
          Tim Hunt added a comment -

          I often use that example question when testing, because copying it and pasting it is quicker than making up your own question, so it has worked until very recently. I also often refer to it in my testing instructions. I just tried re-creating that example question on my dev server, and it worked.

          (What did the testing instructions on MDL-34570 say? Oh, I see. Not very good instructions. That will be why testing did not pick up the regression.)

          I agree that the way multianswer does feedback is not great. You should have seen how badly it used to suck in 1.9 and before. This is not the time to try to fix the long-standing accessability issues there, though if you have any ideas about that later, we would love to know. This is the time to fix the regression you caused, as quickly and unobtrusively as possible.

          It is clear what the JavaScript is trying to do, and why your changes broke it. If I had had time to fix it myself, I would have done so, but I have less than a week before I fly out to Australia, and things to finish before I do. That is why I asked you to think of the solution.

          The one thing that occurs to me now is that <lable> is an inline tag. Therefore, if you just want to replace it with an equivalent tag, <span> should be safe.

          Show
          Tim Hunt added a comment - I often use that example question when testing, because copying it and pasting it is quicker than making up your own question, so it has worked until very recently. I also often refer to it in my testing instructions. I just tried re-creating that example question on my dev server, and it worked. (What did the testing instructions on MDL-34570 say? Oh, I see. Not very good instructions. That will be why testing did not pick up the regression.) I agree that the way multianswer does feedback is not great. You should have seen how badly it used to suck in 1.9 and before. This is not the time to try to fix the long-standing accessability issues there, though if you have any ideas about that later, we would love to know. This is the time to fix the regression you caused, as quickly and unobtrusively as possible. It is clear what the JavaScript is trying to do, and why your changes broke it. If I had had time to fix it myself, I would have done so, but I have less than a week before I fly out to Australia, and things to finish before I do. That is why I asked you to think of the solution. The one thing that occurs to me now is that <lable> is an inline tag. Therefore, if you just want to replace it with an equivalent tag, <span> should be safe.
          Hide
          Frédéric Massart added a comment -

          I see the problem I had with the question sample provided, I did not click on "Decode and verify the question text", I thought this was an optional step.

          This patch fixes the regression introduced.

          Tim, please feel free to peer review it if you can find some time to do so.

          Thanks!

          Show
          Frédéric Massart added a comment - I see the problem I had with the question sample provided, I did not click on "Decode and verify the question text", I thought this was an optional step. This patch fixes the regression introduced. Tim, please feel free to peer review it if you can find some time to do so. Thanks!
          Hide
          Tim Hunt added a comment -

          +1. Thanks Fred.

          Show
          Tim Hunt added a comment - +1. Thanks Fred.
          Hide
          Tõnis Tartes added a comment -

          Oh well.. i tried the 2 changes on my moodle installation and the problem still exists with the new patches applied..

          It still works fine using the label tags.

          Show
          Tõnis Tartes added a comment - Oh well.. i tried the 2 changes on my moodle installation and the problem still exists with the new patches applied.. It still works fine using the label tags.
          Hide
          Frédéric Massart added a comment -

          Have you purged your cache (Moodle and Browser)?

          Show
          Frédéric Massart added a comment - Have you purged your cache (Moodle and Browser)?
          Hide
          Tõnis Tartes added a comment -

          Sorry, my mistake.. it works nice
          I purged only my moodle installations cache and not my browsers cache..

          Thanks for the fix!

          Show
          Tõnis Tartes added a comment - Sorry, my mistake.. it works nice I purged only my moodle installations cache and not my browsers cache.. Thanks for the fix!
          Hide
          Aaron C Spike added a comment -

          I made a quick hack around in our moodle to tide us over until this fix lands. I edited only the js to account for the lack of nesting in the new html.

          https://github.com/martinluther/moodle/commits/multianswerfeedback

          Show
          Aaron C Spike added a comment - I made a quick hack around in our moodle to tide us over until this fix lands. I edited only the js to account for the lack of nesting in the new html. https://github.com/martinluther/moodle/commits/multianswerfeedback
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 and master)

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 and master)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing, the review now shows the feedback (status, correct, marks...) hovering. Not for all subtypes (MCV/MCH show the feedback "inline") but for the rest. I hope that's enough.

          Show
          Eloy Lafuente (stronk7) added a comment - Passing, the review now shows the feedback (status, correct, marks...) hovering. Not for all subtypes (MCV/MCH show the feedback "inline") but for the rest. I hope that's enough.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          From somewhere within the clouds...

          Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - From somewhere within the clouds... Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: