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
    • Rank:
      42140

      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.

        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: