Moodle
  1. Moodle
  2. MDL-34025

Call to close_attempt_popup passes parameters in wrong order.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.4, 2.3.1
    • Component/s: Quiz
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Create a quiz that allows multiple attempts, and set to Browser security: 'Popup ...', and no review allowed immediately after the attempt.
      2. Add a question to the quiz
      3. Create a student account and access the quiz.
      4. Finish an attempt
      5. Ensure a valid message is displayed and the pop-up window then closes after a few seconds.

      Show
      1. Create a quiz that allows multiple attempts, and set to Browser security: 'Popup ...', and no review allowed immediately after the attempt. 2. Add a question to the quiz 3. Create a student account and access the quiz. 4. Finish an attempt 5. Ensure a valid message is displayed and the pop-up window then closes after a few seconds.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34025_master

      Description

      In mod/quiz/accessmanager.php the function back_to_view_page makes a call to close_attempt_popup and passes the message as the first parameter, and the url as the second parameter, when in fact it should be reversed. Function close_attempt_popup is declared in mod/quiz/renderer.php.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Tim Hunt added a comment -

            Oops! That is presumably my mistake. (blush).

            Thanks for the fix. The code change is good.

            Please could you amend it to make the commit comment match the Moodle style (http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages). I would suggest

            MDL-34025 quiz secure window: fix popup closing

            Also, are you going to cherry-pick to the other stable branches and master, and write some testing instructions?

            Thanks.

            Show
            Tim Hunt added a comment - Oops! That is presumably my mistake. (blush). Thanks for the fix. The code change is good. Please could you amend it to make the commit comment match the Moodle style ( http://docs.moodle.org/dev/Commit_cheat_sheet#Provide_clear_commit_messages ). I would suggest MDL-34025 quiz secure window: fix popup closing Also, are you going to cherry-pick to the other stable branches and master, and write some testing instructions? Thanks.
            Hide
            Mark Nelson added a comment -

            Hi Tim,

            It appears this issue is only introduced in Moodle 2.2 onwards, as the earlier versions do not have the same function calls.

            Regards,

            Mark

            Show
            Mark Nelson added a comment - Hi Tim, It appears this issue is only introduced in Moodle 2.2 onwards, as the earlier versions do not have the same function calls. Regards, Mark
            Hide
            Tim Hunt added a comment -

            Thanks Mark. I just edited the testing instructions to add two important settings that I think are required for this to be testable.

            Show
            Tim Hunt added a comment - Thanks Mark. I just edited the testing instructions to add two important settings that I think are required for this to be testable.
            Hide
            Dan Poltawski added a comment -

            Thanks Mark!

            Integrated to master, 22 and 23

            Show
            Dan Poltawski added a comment - Thanks Mark! Integrated to master, 22 and 23
            Hide
            Ankit Agarwal added a comment -

            I get the msg "Your request has been processed. You can now close this window"
            But popup is not closed.
            Tested on chromium/integration master
            Failing this Sorry

            Show
            Ankit Agarwal added a comment - I get the msg "Your request has been processed. You can now close this window" But popup is not closed. Tested on chromium/integration master Failing this Sorry
            Hide
            Dan Poltawski added a comment -

            Ankit, do you get any JS errors?

            Show
            Dan Poltawski added a comment - Ankit, do you get any JS errors?
            Hide
            Ankit Agarwal added a comment - - edited

            I didn't check the console, will retry and see.
            Edit:- console is clean
            Edit2:- Just notcied this. The popup seems to keep loading forever after the attempt is finished and the msg is displayed.

            Thanks

            Show
            Ankit Agarwal added a comment - - edited I didn't check the console, will retry and see. Edit:- console is clean Edit2:- Just notcied this. The popup seems to keep loading forever after the attempt is finished and the msg is displayed. Thanks
            Hide
            Tim Hunt added a comment -

            A quick Google finds http://stackoverflow.com/questions/2032640/problem-with-window-close-and-chrome

            Note that this bug fix was fixing the call to the close_attempt_popup API. If the implementation of that method is broken, then that should not block integration of this fix (although it would be good to get it fixed too) (and get the fix in this week).

            Show
            Tim Hunt added a comment - A quick Google finds http://stackoverflow.com/questions/2032640/problem-with-window-close-and-chrome Note that this bug fix was fixing the call to the close_attempt_popup API. If the implementation of that method is broken, then that should not block integration of this fix (although it would be good to get it fixed too) (and get the fix in this week).
            Hide
            Dan Poltawski added a comment -

            Sounds sensible - passing.

            Ankit, could you please raise an issue for it?

            Show
            Dan Poltawski added a comment - Sounds sensible - passing. Ankit, could you please raise an issue for it?
            Hide
            Ankit Agarwal added a comment -

            Created MDL-34187 for the same.
            Thanks

            Show
            Ankit Agarwal added a comment - Created MDL-34187 for the same. Thanks
            Hide
            Sam Hemelryk added a comment -

            Congratulations your code is upstream - gold star for you!

            This issue + 79 others made it in in time for the minor releases.
            Thank you everyone involved for your exuberant efforts.

            Show
            Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: