Moodle
  1. Moodle
  2. MDL-28424

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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:
    • Rank:
      18156

      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()

        Activity

        Brigitte Baquero created issue -
        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)
        Tim Hunt made changes -
        Fix Version/s STABLE backlog [ 10463 ]
        Labels triaged
        Hide
        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
        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
        Helen Foster added a comment -

        Increasing priority as suggested.

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

        Reviewing an affected quiz through the gradebook gives the same error

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

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

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

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

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

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

        Show
        Matthew Davidson added a comment - We are getting tons of teachers asking about this. Any movement on this bug?
        Hide
        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
        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
        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
        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
        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
        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
        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
        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.
        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
        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
        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
        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
        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
        Luis de Vasconcelos added a comment - - edited

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

        Show
        Luis de Vasconcelos added a comment - - edited Is this problem related to MDL-23706 (and MDL-26746 )?
        Hide
        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
        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
        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
        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
        Mike Churchward added a comment -

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

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

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

        Show
        Dixie Harrison added a comment - I would also be more than happy to help with testing.
        Hide
        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
        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.)
        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
        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 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 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
        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
        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.
        Tim Hunt made changes -
        Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
        Hide
        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
        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.
        Tim Hunt made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Tim Hunt made changes -
        Peer reviewer mchurch
        Hide
        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
        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 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 Harrison added a comment - - edited My test was successful. Thank you! You just got me out of hot water with a ton of instructors.
        Eloy Lafuente (stronk7) made changes -
        Labels triaged triaged yay!
        Sam Hemelryk made changes -
        Currently in integration Yes
        Sam Hemelryk made changes -
        Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
        Integrator samhemelryk
        Hide
        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
        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
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Please can you remove the unused global during integration. Thanks, and Sorry.
        Hide
        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 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
        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
        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
        Sam Hemelryk added a comment -

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

        Cheers
        Sam

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

        Thank you.

        Show
        Tim Hunt added a comment - Thank you.
        Rossiani Wijaya made changes -
        Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
        Tester rwijaya
        Hide
        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
        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
        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
        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
        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
        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
        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
        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.
        Rossiani Wijaya made changes -
        Status Testing in progress [ 10011 ] Tested [ 10006 ]
        Hide
        Koen Roggemans added a comment -

        Thank you all for fixing this - very much appreciated!

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

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

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

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

        Show
        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:
            22 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: