Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-36422

Accessibility bug in Quiz when trying to do manual grading

    Details

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

      1. Create a test with couple of questions
      2. Attempt the test with some student accounts
      3. Login as teacher and try to do manual grading on the attempts
      4. Make sure the HTML is correctly displayed
      5. Try on a few different browsers

      Show
      1. Create a test with couple of questions 2. Attempt the test with some student accounts 3. Login as teacher and try to do manual grading on the attempts 4. Make sure the HTML is correctly displayed 5. Try on a few different browsers
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36422-master

      Description

      Found another regression related from http://tracker.moodle.org/browse/MDL-34570 in the latest MOODLE_23_STABLE release.
      If you open up any test with some attempts and try to do manual grading, then the UI view is messed up.

      Note: This happens in latest weekly release, not in the 2.3.2 release from September.
      Any weekly release after MDL-34570 has been integrated.

        Gliffy Diagrams

          Attachments

          1. binarious_theme2.jpg
            binarious_theme2.jpg
            50 kB
          2. binarius_theme1.jpg
            binarius_theme1.jpg
            102 kB
          3. custom_theme1.jpg
            custom_theme1.jpg
            124 kB
          4. custom_theme2.jpg
            custom_theme2.jpg
            90 kB

            Issue Links

              Activity

              Hide
              t6nis20 Tõnis Tartes added a comment - - edited

              I also found the cause of that..
              question/behaviour/rendererbase.php around line 89 -

                      $commenteditor = html_writer::tag('div', html_writer::tag('textarea', s($commenttext),
                              array('id' => $id, 'name' => $inputname, 'rows' => 10, 'cols' => 60)));
                      $commenteditor .= html_writer::end_tag('div');

              I think this line is unneccessary - $commenteditor .= html_writer::end_tag('div'); - because html_writer::tag automatically closes the div.
              If i commented that line out, the view was fixed.

              gitlog link - https://github.com/moodle/moodle/commit/d3187c299a14659bd5ec5ccc3e4a243ba3659583

              Show
              t6nis20 Tõnis Tartes added a comment - - edited I also found the cause of that.. question/behaviour/rendererbase.php around line 89 - $commenteditor = html_writer::tag('div', html_writer::tag('textarea', s($commenttext), array('id' => $id, 'name' => $inputname, 'rows' => 10, 'cols' => 60))); $commenteditor .= html_writer::end_tag('div'); I think this line is unneccessary - $commenteditor .= html_writer::end_tag('div'); - because html_writer::tag automatically closes the div. If i commented that line out, the view was fixed. gitlog link - https://github.com/moodle/moodle/commit/d3187c299a14659bd5ec5ccc3e4a243ba3659583
              Hide
              timhunt Tim Hunt added a comment -

              I think your analysis of the problem is wrong. That end_tag matches a corresponding start_tag higher up the code. Also, that end_tag line was not changed as part of MDL-34570.

              Show
              timhunt Tim Hunt added a comment - I think your analysis of the problem is wrong. That end_tag matches a corresponding start_tag higher up the code. Also, that end_tag line was not changed as part of MDL-34570 .
              Hide
              timhunt Tim Hunt added a comment -

              Fred, any chance you could take a quick look at this regression? It would be good to fix it before 2.3.3 if that is possible.

              Show
              timhunt Tim Hunt added a comment - Fred, any chance you could take a quick look at this regression? It would be good to fix it before 2.3.3 if that is possible.
              Hide
              t6nis20 Tõnis Tartes added a comment -

              I could'nt find any start_tag before that end_tag inside /question/behaviour/rendererbase.php, thats why i suspect that line as the cause of this problem..
              Maybe it was just accidentally added or maybe indeed the start_tag begins from another file which would make finding it harder i guess..
              Although it would be nice if this would be fixed before 2.3.3, probably more people will find this bug and complain that the blocks are misplaced..

              Show
              t6nis20 Tõnis Tartes added a comment - I could'nt find any start_tag before that end_tag inside /question/behaviour/rendererbase.php, thats why i suspect that line as the cause of this problem.. Maybe it was just accidentally added or maybe indeed the start_tag begins from another file which would make finding it harder i guess.. Although it would be nice if this would be fixed before 2.3.3, probably more people will find this bug and complain that the blocks are misplaced..
              Hide
              fred Frédéric Massart added a comment -

              There you go. I removed the closing </div>. Thanks for reporting this Tonis, I have to admit that on my installation the difference is not visible, but the HTML was incorrect so I fixed it.

              Could you please provide a screenshot of you are experiencing? Along with some eventual settings that you might have set (or themes). That might help improving the testing instructions.

              Cheers!
              Fred

              Show
              fred Frédéric Massart added a comment - There you go. I removed the closing </div>. Thanks for reporting this Tonis, I have to admit that on my installation the difference is not visible, but the HTML was incorrect so I fixed it. Could you please provide a screenshot of you are experiencing? Along with some eventual settings that you might have set (or themes). That might help improving the testing instructions. Cheers! Fred
              Hide
              poltawski Dan Poltawski added a comment -

              Expedited integration review because Fred asked nicely on behalf of Tim (and its easy )

              Show
              poltawski Dan Poltawski added a comment - Expedited integration review because Fred asked nicely on behalf of Tim (and its easy )
              Hide
              poltawski Dan Poltawski added a comment -

              Intgerated to 22, 23 and master. Thanks Fred

              Show
              poltawski Dan Poltawski added a comment - Intgerated to 22, 23 and master. Thanks Fred
              Hide
              timhunt Tim Hunt added a comment -

              Thanks all.

              Show
              timhunt Tim Hunt added a comment - Thanks all.
              Hide
              markn Mark Nelson added a comment -

              Page looked fine to me, no breakage here.

              Show
              markn Mark Nelson added a comment - Page looked fine to me, no breakage here.
              Hide
              t6nis20 Tõnis Tartes added a comment -

              Attaching couple of screenshots.
              2 Screenshots from a custom theme and couple of screenshots from one Moodles default theme.

              Show
              t6nis20 Tõnis Tartes added a comment - Attaching couple of screenshots. 2 Screenshots from a custom theme and couple of screenshots from one Moodles default theme.
              Hide
              t6nis20 Tõnis Tartes added a comment -

              With the fix applied, it shows everything correctly.

              Show
              t6nis20 Tõnis Tartes added a comment - With the fix applied, it shows everything correctly.
              Hide
              fred Frédéric Massart added a comment -

              Thanks for testing this Mark and Tonis!

              Show
              fred Frédéric Massart added a comment - Thanks for testing this Mark and Tonis!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

              (not really)

              Closing, thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    12/Nov/12