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

Quiz question export fails when slasharguments is turned off

    Details

    • Testing Instructions:
      Hide

      1. Turn of slasharguments in the admin settings.

      2. Go to the question bank and export some questions.

      3. Make sure the download starts automatically.

      Show
      1. Turn of slasharguments in the admin settings. 2. Go to the question bank and export some questions. 3. Make sure the download starts automatically.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      To reproduce:

      1. Make sure slasharguments is turned on.
      2. Create a question bank and populate it.
      3. Export the questions.
      4. Turn off slasharguments.
      5. Go back to the question bank.
      6. Export the questions.

      The last step causes a stack trace:

      Default exception handler: No valid arguments supplied or incorrect server configuration Debug: 
      Error code: invalidargorconf
      * line 467 of /lib/setuplib.php: moodle_exception thrown
      * line 3422 of /lib/filelib.php: call to print_error()
      * line 38 of /pluginfile.php: call to file_pluginfile()

      This may be the same issue as MDL-33116.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            I don't believe the cause is the same as MDL-33116.

            As far as I can see, the question code is doing the right thing. question_make_export_url in questionlib.php is calling moodle_url::make_file_url, but then pluginfile.php fails to handle that URL correctly.

            Therefore, re-assigning to the File API component.

            Show
            timhunt Tim Hunt added a comment - I don't believe the cause is the same as MDL-33116 . As far as I can see, the question code is doing the right thing. question_make_export_url in questionlib.php is calling moodle_url::make_file_url, but then pluginfile.php fails to handle that URL correctly. Therefore, re-assigning to the File API component.
            Hide
            timhunt Tim Hunt added a comment -

            Michel, this seems to be more a Files problem than a Questions problem, so please can you triage it.

            Show
            timhunt Tim Hunt added a comment - Michel, this seems to be more a Files problem than a Questions problem, so please can you triage it.
            Hide
            salvetore Michael de Raadt added a comment -

            No worries. Thanks for bringing this to my attention, Tim.

            Show
            salvetore Michael de Raadt added a comment - No worries. Thanks for bringing this to my attention, Tim.
            Hide
            ohameiri omer added a comment -

            Hey,

            Any news on this matter ?

            Thanks,
            Omer

            Show
            ohameiri omer added a comment - Hey, Any news on this matter ? Thanks, Omer
            Hide
            brianking Brian King added a comment - - edited

            Maybe the documentation is wrong, but the phpdocs for moodle_url::out() say
            "If you use the returned URL in HTML code, you want the escaped ampersands. If you use the returned URL in HTTP headers, you want $escaped=false."

            This patch will fix the issue, as far as i can see:

            --- a/question/export.php
            +++ b/question/export.php
            @@ -73,7 +73,7 @@ if ($from_form = $export_form->get_data()) {
                 echo get_string('yourfileshoulddownload', 'question', $export_url->out());
                 echo $OUTPUT->box_end();
             
            -    $PAGE->requires->js_function_call('document.location.replace', array($export_url->out()), false, 1);
            +    $PAGE->requires->js_function_call('document.location.replace', array($export_url->out(false)), false, 1);
             
                 echo $OUTPUT->continue_button(new moodle_url('edit.php', $thispageurl->params()));
                 echo $OUTPUT->footer();

            Show
            brianking Brian King added a comment - - edited Maybe the documentation is wrong, but the phpdocs for moodle_url::out() say "If you use the returned URL in HTML code, you want the escaped ampersands. If you use the returned URL in HTTP headers, you want $escaped=false." This patch will fix the issue, as far as i can see: --- a/question/export.php +++ b/question/export.php @@ -73,7 +73,7 @@ if ($from_form = $export_form->get_data()) { echo get_string('yourfileshoulddownload', 'question', $export_url->out()); echo $OUTPUT->box_end(); - $PAGE->requires->js_function_call('document.location.replace', array($export_url->out()), false, 1); + $PAGE->requires->js_function_call('document.location.replace', array($export_url->out(false)), false, 1); echo $OUTPUT->continue_button(new moodle_url('edit.php', $thispageurl->params())); echo $OUTPUT->footer();
            Hide
            ohameiri omer added a comment -

            Thanks, I'll test it.

            if this is all it takes, why isnt this closed ?

            Show
            ohameiri omer added a comment - Thanks, I'll test it. if this is all it takes, why isnt this closed ?
            Hide
            timhunt Tim Hunt added a comment -

            Brian, thank you for working that out! I guess the PHPdoc comment does not address the case of "if you are passing the URL to JavaScript ..." at all.

            Are you familiar with the process to prepare a patch for integration, or shall I do it?

            omer, Until Brian did the clever detective work to determine that is all that it would take, we did not know that. Comments like yours do not help get the bug fixed.

            Show
            timhunt Tim Hunt added a comment - Brian, thank you for working that out! I guess the PHPdoc comment does not address the case of "if you are passing the URL to JavaScript ..." at all. Are you familiar with the process to prepare a patch for integration, or shall I do it? omer, Until Brian did the clever detective work to determine that is all that it would take, we did not know that. Comments like yours do not help get the bug fixed.
            Hide
            ohameiri omer added a comment -

            Tim,
            You misunderstood me.

            I'll write this again, to make it clear:

            "
            if this is all it takes, why isnt this closed ?
            Am i missing something ?
            "

            Omer

            Show
            ohameiri omer added a comment - Tim, You misunderstood me. I'll write this again, to make it clear: " if this is all it takes, why isnt this closed ? Am i missing something ? " Omer
            Hide
            timhunt Tim Hunt added a comment -

            I have now made a commit and submitted it for integration.

            Show
            timhunt Tim Hunt added a comment - I have now made a commit and submitted it for integration.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated, thanks everyone.

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks everyone.
            Hide
            fred Frédéric Massart added a comment -

            Test passed on 2.2, 2.3 and 2.4. Thanks!

            Show
            fred Frédéric Massart added a comment - Test passed on 2.2, 2.3 and 2.4. Thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/Jan/13