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

Quiz with Browser Security enabled don't shows results (Moodle 2.1)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.1.2
    • Component/s: Quiz
    • Labels:
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      1. Create a quiz with default settings.
      2. Make sure you can preview it as a teacher.
      3. Make sure that you can attempt and review it as a student.
      4. Make sure that the teacher can review the students' attempt.

      5 - 8. Ditto, but for a quiz with Browser security set to Full screen pop-up with some JavaScript security

      Show
      1. Create a quiz with default settings. 2. Make sure you can preview it as a teacher. 3. Make sure that you can attempt and review it as a student. 4. Make sure that the teacher can review the students' attempt. 5 - 8. Ditto, but for a quiz with Browser security set to Full screen pop-up with some JavaScript security
    • Workaround:
      Hide

      Just taking Matt's comment and adding it as a workaround with some minor tweaks: With browser security (i.e. mdl_quiz table's popup field) set to true, students cannot review the quiz or make multiple attempts. By going to the quiz settings, the teacher can switch Browser security to "none" and then students are able to review the quiz and make multiple attempts. If a site admin wanted to change this for all quizzes, they could set the popup field to 0 for all quizzes. Peace - Anthony

      Show
      Just taking Matt's comment and adding it as a workaround with some minor tweaks: With browser security (i.e. mdl_quiz table's popup field) set to true, students cannot review the quiz or make multiple attempts. By going to the quiz settings, the teacher can switch Browser security to "none" and then students are able to review the quiz and make multiple attempts. If a site admin wanted to change this for all quizzes, they could set the popup field to 0 for all quizzes. Peace - Anthony
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      If a quiz has the "Browser security" option in "Full screen pop-up with some JavaScript security", after completing a quiz, instead of showing the results, it shows this error:

      Coding error detected, it must be fixed by a programmer: block_manager has already prepared the blocks in region side-prefor output. It is too late to add fake block.

      If you hit continue, it scores the quiz correctly in the gradebook, but the results are never shown on screen.

      When the debugging option in enabled, it shows this error:

      line 425 of /lib/blocklib.php: coding_exception thrown

      line 257 of /mod/quiz/review.php call to block_manager->add_fake_block()

        Gliffy Diagrams

          Activity

          brigittebaquero Brigitte Baquero created issue -
          brigittebaquero Brigitte Baquero made changes -
          Field Original Value New Value
          Summary Quiz with Brower Security enabled don't shows results (Moodle 2.1) Quiz with Browser Security enabled don't shows results (Moodle 2.1)
          timhunt Tim Hunt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Labels triaged
          Hide
          evanirvingpease Evan Irving-Pease added a comment -

          Is a priority of "minor" really appropriate for this issue?

          The error is a fatal one which prevents the user from seeing their feedback. Surely this is a "major" bug. It makes the quiz unusable with browser security turned on.

          Show
          evanirvingpease Evan Irving-Pease added a comment - Is a priority of "minor" really appropriate for this issue? The error is a fatal one which prevents the user from seeing their feedback. Surely this is a "major" bug. It makes the quiz unusable with browser security turned on.
          Hide
          tsala Helen Foster added a comment -

          Increasing priority as suggested.

          Show
          tsala Helen Foster added a comment - Increasing priority as suggested.
          tsala Helen Foster made changes -
          Priority Minor [ 4 ] Major [ 3 ]
          Hide
          koen Koen Roggemans added a comment -

          Reviewing an affected quiz through the gradebook gives the same error

          Show
          koen Koen Roggemans added a comment - Reviewing an affected quiz through the gradebook gives the same error
          Hide
          glenmontes Glen Antonio Montes added a comment -

          Some progress about this? Surely this is a "major" bug. Thanks

          Show
          glenmontes Glen Antonio Montes added a comment - Some progress about this? Surely this is a "major" bug. Thanks
          Hide
          smithrn Ryan Smith added a comment -

          Any progress on this? Our users are now discovering this bug when they enable browser security.

          Show
          smithrn Ryan Smith added a comment - Any progress on this? Our users are now discovering this bug when they enable browser security.
          Hide
          syxton Matthew Davidson added a comment -

          We are getting tons of teachers asking about this. Any movement on this bug?

          Show
          syxton Matthew Davidson added a comment - We are getting tons of teachers asking about this. Any movement on this bug?
          Hide
          timhunt Tim Hunt added a comment -

          Sorry no. I know this is important to a lot of people, but so are a million other things on my todo list.

          Show
          timhunt Tim Hunt added a comment - Sorry no. I know this is important to a lot of people, but so are a million other things on my todo list.
          Hide
          aborrow Anthony Borrow added a comment -

          Tim - Do regressions get any increased priority. This is functionality that was working and now is not working. Is there anything (short of a patch) that I can do to help? It has something to do with the way the fake block is being created in the context of the "secure" page. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Tim - Do regressions get any increased priority. This is functionality that was working and now is not working. Is there anything (short of a patch) that I can do to help? It has something to do with the way the fake block is being created in the context of the "secure" page. Peace - Anthony
          Hide
          timhunt Tim Hunt added a comment -

          Regressions do get slightly higher priority, but that still does not get this anywhere near the top of my todo list.

          A patch is certainly the most likely way for this to get fixed.

          Failing that, an analysis of what exactly is going wrong would help, but if you get that far, I am sure you could make the patch yourself.

          Show
          timhunt Tim Hunt added a comment - Regressions do get slightly higher priority, but that still does not get this anywhere near the top of my todo list. A patch is certainly the most likely way for this to get fixed. Failing that, an analysis of what exactly is going wrong would help, but if you get that far, I am sure you could make the patch yourself.
          Hide
          mtrost Matt Trost added a comment -

          With security enabled, students cannot review the quiz or make multiple attempts. By Switching the security to "none", students are able to review the quiz and make multiple attempts.

          Show
          mtrost Matt Trost added a comment - With security enabled, students cannot review the quiz or make multiple attempts. By Switching the security to "none", students are able to review the quiz and make multiple attempts.
          aborrow Anthony Borrow made changes -
          Workaround Just taking Matt's comment and adding it as a workaround with some minor tweaks: With browser security (i.e. mdl_quiz table's popup field) set to true, students cannot review the quiz or make multiple attempts. By going to the quiz settings, the teacher can switch Browser security to "none" and then students are able to review the quiz and make multiple attempts. If a site admin wanted to change this for all quizzes, they could set the popup field to 0 for all quizzes. Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          Just for the benefit of those watching this issue, there have been numerous discussions in the forums about the relative value of taking measures to prevent students from copying and pasting quiz questions. Given the social constructionist pedagogy underlying Moodle, preventing access to information (while sometimes justified) often seems in my own personal estimation to be slightly misguided. It is a matter of debate and I think that this issue should be fixed so that it behaves as it did before but I agree with Tim that there are many other tasks as well. I'll see if I can go in and sort things out about what is really happening but it will involve me taking some time to learn about how pages are constructed post 2.0. It may be a quick job for someone like Sam. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Just for the benefit of those watching this issue, there have been numerous discussions in the forums about the relative value of taking measures to prevent students from copying and pasting quiz questions. Given the social constructionist pedagogy underlying Moodle, preventing access to information (while sometimes justified) often seems in my own personal estimation to be slightly misguided. It is a matter of debate and I think that this issue should be fixed so that it behaves as it did before but I agree with Tim that there are many other tasks as well. I'll see if I can go in and sort things out about what is really happening but it will involve me taking some time to learn about how pages are constructed post 2.0. It may be a quick job for someone like Sam. Peace - Anthony
          Hide
          michaelp Michael Penney added a comment -

          Importance of the feature aside - a feature that exists in the UI that is broken leads to users distrusting other parts of the system.

          It's important for users to trust the Quiz module because they make high stakes decisions based on it's results.

          If the feature is not going to be fixed right away, it would be ideal to remove/hide it so that users don't try to use it and find it broken.

          Show
          michaelp Michael Penney added a comment - Importance of the feature aside - a feature that exists in the UI that is broken leads to users distrusting other parts of the system. It's important for users to trust the Quiz module because they make high stakes decisions based on it's results. If the feature is not going to be fixed right away, it would be ideal to remove/hide it so that users don't try to use it and find it broken.
          Hide
          libertymoodle Luis de Vasconcelos added a comment - - edited

          Is this problem related to MDL-23706 (and MDL-26746)?

          Show
          libertymoodle Luis de Vasconcelos added a comment - - edited Is this problem related to MDL-23706 (and MDL-26746 )?
          Hide
          mchurch Mike Churchward added a comment -

          Trying to do some debugging of this problem... It appears that the problem stems from output having been sent already by the time the "add_fake_block" function is called. Tim, do you know off-hand where that would have been started when the quiz is in secure mode vs. not in secure mode?

          Show
          mchurch Mike Churchward added a comment - Trying to do some debugging of this problem... It appears that the problem stems from output having been sent already by the time the "add_fake_block" function is called. Tim, do you know off-hand where that would have been started when the quiz is in secure mode vs. not in secure mode?
          Hide
          timhunt Tim Hunt added a comment -

          Almost certainly at line 104

          $accessmanager->setup_secure_page(...

          I think the problem is that securewindow_access_rule::setup_secure_page ends with

          echo $OUTPUT->header();

          The setup method should not do any output, so we should delete that from there, and then review all the pages that call this method.

          I understand this issue now, and fixing it properly would clean up a lot of mess. I should be able to do the fix tomorrow, for integration next week, particularly if some people can help me with the testing.

          Show
          timhunt Tim Hunt added a comment - Almost certainly at line 104 $accessmanager->setup_secure_page(... I think the problem is that securewindow_access_rule::setup_secure_page ends with echo $OUTPUT->header(); The setup method should not do any output, so we should delete that from there, and then review all the pages that call this method. I understand this issue now, and fixing it properly would clean up a lot of mess. I should be able to do the fix tomorrow, for integration next week, particularly if some people can help me with the testing.
          Hide
          mchurch Mike Churchward added a comment -

          Happy to help with testing. Let me know when its ready.

          Show
          mchurch Mike Churchward added a comment - Happy to help with testing. Let me know when its ready.
          Hide
          dixie Dixie Harrison added a comment -

          I would also be more than happy to help with testing.

          Show
          dixie Dixie Harrison added a comment - I would also be more than happy to help with testing.
          Hide
          timhunt Tim Hunt added a comment -

          OK people, please test this. Please text both secure and non-secure quizzes, since the changes may affect both. (Obviously, there should be no changes for non-secure quizzes.)

          Show
          timhunt Tim Hunt added a comment - OK people, please test this. Please text both secure and non-secure quizzes, since the changes may affect both. (Obviously, there should be no changes for non-secure quizzes.)
          timhunt Tim Hunt made changes -
          Status Open [ 1 ] Waiting for peer review [ 10012 ]
          Pull Master Diff URL https://github.com/timhunt/moodle/compare/master...MDL-28424
          Pull Master Branch MDL-28424
          Pull from Repository git://github.com/timhunt/moodle.git
          Pull 2.1 Branch MDL-28424_21
          Pull 2.1 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-28424_21
          timhunt Tim Hunt made changes -
          Fix Version/s 2.1.2 [ 10851 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Testing Instructions 1. Create a quiz with default settings.
          2. Make sure you can preview it as a teacher.
          3. Make sure that you can attempt and review it as a student.
          4. Make sure that the teacher can review the students' attempt.

          5 - 8. Ditto, but for a quiz with Browser security set to Full screen pop-up with some JavaScript security
          Hide
          dixie Dixie Harrison added a comment -

          The message I got back was
          Coding error detected, it must be fixed by a programmer: Invalid state passed to moodle_page::set_state. We are in state 2 and state 1 was requested.

          I have a meeting to go to and I will try again after that as I don't have git on my server and had to patch the files manually - which means I could have done something I shouldn't have. Or not done something I should have.

          Show
          dixie Dixie Harrison added a comment - The message I got back was Coding error detected, it must be fixed by a programmer: Invalid state passed to moodle_page::set_state. We are in state 2 and state 1 was requested. I have a meeting to go to and I will try again after that as I don't have git on my server and had to patch the files manually - which means I could have done something I shouldn't have. Or not done something I should have.
          Hide
          mchurch Mike Churchward added a comment -

          I tested your changes on both a secure and non-secure quiz. The problems documented here were resolved, and I could not find any other problems. I could not replicate the problem reported with the gradebook either before or after the changes.

          Show
          mchurch Mike Churchward added a comment - I tested your changes on both a secure and non-secure quiz. The problems documented here were resolved, and I could not find any other problems. I could not replicate the problem reported with the gradebook either before or after the changes.
          timhunt Tim Hunt made changes -
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          Hide
          timhunt Tim Hunt added a comment -

          Thanks Mike. That is good enough for me. Submitting for integration.

          However, if anyone else can test this between now and Monday, when this is integrated, that would help ensure that there are no un-pleasant side-effects to this fix. Thanks.

          Show
          timhunt Tim Hunt added a comment - Thanks Mike. That is good enough for me. Submitting for integration. However, if anyone else can test this between now and Monday, when this is integrated, that would help ensure that there are no un-pleasant side-effects to this fix. Thanks.
          timhunt Tim Hunt made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          timhunt Tim Hunt made changes -
          Peer reviewer mchurch
          Hide
          aborrow Anthony Borrow added a comment -

          Thanks Tim and Mike - I also have tested it and the patch seemed to fix things up just fine. I did not find any unpleasant side effects. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Thanks Tim and Mike - I also have tested it and the patch seemed to fix things up just fine. I did not find any unpleasant side effects. Peace - Anthony
          Hide
          dixie Dixie Harrison added a comment - - edited

          My test was successful.

          Thank you! You just got me out of hot water with a ton of instructors.

          Show
          dixie Dixie Harrison added a comment - - edited My test was successful. Thank you! You just got me out of hot water with a ton of instructors.
          stronk7 Eloy Lafuente (stronk7) made changes -
          Labels triaged triaged yay!
          samhemelryk Sam Hemelryk made changes -
          Currently in integration Yes
          samhemelryk Sam Hemelryk made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator samhemelryk
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi Tim,

          The changes look good however you've left being an unused global, securewindow_access_rule::setup_secure_page no longer uses $OUTPUT.
          Would you like to fix it up or are you happy for me to remove it during the integration merge?

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Tim, The changes look good however you've left being an unused global, securewindow_access_rule::setup_secure_page no longer uses $OUTPUT. Would you like to fix it up or are you happy for me to remove it during the integration merge? Cheers Sam
          Hide
          timhunt Tim Hunt added a comment -

          Please can you remove the unused global during integration. Thanks, and Sorry.

          Show
          timhunt Tim Hunt added a comment - Please can you remove the unused global during integration. Thanks, and Sorry.
          Hide
          francis.feytout Francis Feytout added a comment -

          So is the fix in today's weekly build ? If not how can I get to test if ?
          (i'm new to the bug tracker)

          Show
          francis.feytout Francis Feytout added a comment - So is the fix in today's weekly build ? If not how can I get to test if ? (i'm new to the bug tracker)
          Hide
          timhunt Tim Hunt added a comment -

          The key field to look at is the 'Status: ' at the top of this bug report. This is currently going through the Waiting for integration review -> Integration review in progress -> Waiting for testing -> Tested process. If all that works out, it will be included in this week's weekly build, and the bug will then be marked closed. (See http://docs.moodle.org/dev/Process for more details.)

          If you want to get the fix early to test it, you need to look at the fields like Pull 2.1 Diff URL: https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-28424_21

          That shows you what the changes are. If you acutally want to make the changes in your version of Moodle it is easiest if you know how to use the git version control system.

          Show
          timhunt Tim Hunt added a comment - The key field to look at is the 'Status: ' at the top of this bug report. This is currently going through the Waiting for integration review -> Integration review in progress -> Waiting for testing -> Tested process. If all that works out, it will be included in this week's weekly build, and the bug will then be marked closed. (See http://docs.moodle.org/dev/Process for more details.) If you want to get the fix early to test it, you need to look at the fields like Pull 2.1 Diff URL: https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-28424_21 That shows you what the changes are. If you acutally want to make the changes in your version of Moodle it is easiest if you know how to use the git version control system.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Tim, this has been integrated now and I fixed up the global during the merge.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Tim, this has been integrated now and I fixed up the global during the merge. Cheers Sam
          samhemelryk Sam Hemelryk made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Hide
          timhunt Tim Hunt added a comment -

          Thank you.

          Show
          timhunt Tim Hunt added a comment - Thank you.
          rwijaya Rossiani Wijaya made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester rwijaya
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi Tim,

          The patch works great.

          However, while testing this issue with "Full screen pop-up with some JavaScript security", I noticed that student could click on the browser's close button and a pop-up "start attempt" window still available. This means a student can view all the quiz questions, close the windows, cancel the attempt, and re-attempt the quiz. I'm not sure if this should be reported as different issue.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi Tim, The patch works great. However, while testing this issue with "Full screen pop-up with some JavaScript security", I noticed that student could click on the browser's close button and a pop-up "start attempt" window still available. This means a student can view all the quiz questions, close the windows, cancel the attempt, and re-attempt the quiz. I'm not sure if this should be reported as different issue.
          Hide
          timhunt Tim Hunt added a comment -

          I don't believe you can actually do that. I don't think the Cancel button would cancel the attempt once it has been started. It will just hide the Start attempt dialogue. Did you actually try it to see what happened?

          Show
          timhunt Tim Hunt added a comment - I don't believe you can actually do that. I don't think the Cancel button would cancel the attempt once it has been started. It will just hide the Start attempt dialogue. Did you actually try it to see what happened?
          Hide
          rwijaya Rossiani Wijaya added a comment -

          When I cancel the attempt, it sets the quiz status to "in progress" with "continue the last attempt" button. And if I clicked on "start attempt", the quiz is not open in the new page.

          The pop-up window will worked again after reloading the quiz page.

          Show
          rwijaya Rossiani Wijaya added a comment - When I cancel the attempt, it sets the quiz status to "in progress" with "continue the last attempt" button. And if I clicked on "start attempt", the quiz is not open in the new page. The pop-up window will worked again after reloading the quiz page.
          Hide
          rwijaya Rossiani Wijaya added a comment -

          My comment regarding 'start attempt' with "Full screen pop-up with some JavaScript security" setting is not related to this issue. Therefore I'm going to pass this issue.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - My comment regarding 'start attempt' with "Full screen pop-up with some JavaScript security" setting is not related to this issue. Therefore I'm going to pass this issue. Test passed.
          rwijaya Rossiani Wijaya made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          koen Koen Roggemans added a comment -

          Thank you all for fixing this - very much appreciated!

          Show
          koen Koen Roggemans added a comment - Thank you all for fixing this - very much appreciated!
          stronk7 Eloy Lafuente (stronk7) made changes -
          Affects Version/s 2.2 [ 10656 ]
          Hide
          nebgor Aparup Banerjee added a comment -

          fixes have been rolled merrily up the stream! Thanks everybody!

          Show
          nebgor Aparup Banerjee added a comment - fixes have been rolled merrily up the stream! Thanks everybody!
          nebgor Aparup Banerjee made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes
          Integration date 28/Sep/11
          dougiamas Martin Dougiamas made changes -
          Labels triaged yay! triaged
          Hide
          bremoodleadmin Chris added a comment -

          Hi There,

          I am running v2.0 (default plug-ins) and also get this problem. Will this patch work for me too or do I need to upgrade to 2.1?

          We have very little support where I am, and I'm not a developer so any help on how I can download the patch and instal it would also be very much welcome. Thanks

          Show
          bremoodleadmin Chris added a comment - Hi There, I am running v2.0 (default plug-ins) and also get this problem. Will this patch work for me too or do I need to upgrade to 2.1? We have very little support where I am, and I'm not a developer so any help on how I can download the patch and instal it would also be very much welcome. Thanks
          Hide
          koen Koen Roggemans added a comment -

          Hi Chris,
          It's fixed in version 2.1.2, it never has been reported as broken in 2.0. If you're on a standard Moodle, I think upgrading to 2.1 is easier then applying the patch. Support for 2.0 is ended last month completely, so upgrading is a good idea anyway.

          Show
          koen Koen Roggemans added a comment - Hi Chris, It's fixed in version 2.1.2, it never has been reported as broken in 2.0. If you're on a standard Moodle, I think upgrading to 2.1 is easier then applying the patch. Support for 2.0 is ended last month completely, so upgrading is a good idea anyway.
          Hide
          bremoodleadmin Chris added a comment -

          Thank you very much for your quick response. I will definately go with the upgrade as advised.

          Show
          bremoodleadmin Chris added a comment - Thank you very much for your quick response. I will definately go with the upgrade as advised.

            People

            • Votes:
              32 Vote for this issue
              Watchers:
              21 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                10/Oct/11