Moodle

Accessibility bug in Quiz when trying to do manual grading

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical 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
  • Pull Master Diff URL:

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.

  1. binarious_theme2.jpg
    50 kB
    08/Nov/12 3:56 PM
  2. binarius_theme1.jpg
    102 kB
    08/Nov/12 3:56 PM
  3. custom_theme1.jpg
    124 kB
    08/Nov/12 3:56 PM
  4. custom_theme2.jpg
    90 kB
    08/Nov/12 3:56 PM

Issue Links

is a regression caused by

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDL-34570 Add label for select and text input elements in question

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
is duplicated by

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37402 Quiz Manual Grading display issues

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
Dan Poltawski added a comment -

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

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

Intgerated to 22, 23 and master. Thanks Fred

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

Thanks all.

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

Page looked fine to me, no breakage here.

Show
Mark Nelson added a comment - Page looked fine to me, no breakage here.
Hide
Permalink
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
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
Permalink
Tõnis Tartes added a comment -

With the fix applied, it shows everything correctly.

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

Thanks for testing this Mark and Tonis!

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

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

(not really)

Closing, thanks!

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

People

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: