quiz & qbank MDL-24453 fix all the ways you can edit a question, so that that returnurl paramterer acutally works.

I hope. Please test thoroughly.

mod/quiz/attempt.php

@@ -27,7 +27,7 @@
     $attemptid = required_param('attempt', PARAM_INT);
     $page = optional_param('page', 0, PARAM_INT);
 
-    $url = new moodle_url('/mod/quiz/attempt.php', array('attempt'=>$attemptid));
+    $url = new moodle_url('/mod/quiz/attempt.php', array('attempt' => $attemptid));
     if ($page !== 0) {
         $url->param('page', $page);
     }

mod/quiz/attemptlib.php

@@ -806,13 +806,15 @@ class quiz_attempt extends quiz {
      * @param string $thispageurl the URL of the page this question is being printed on.
      */
     public function print_question($id, $reviewing, $thispageurl = '') {
+        global $CFG;
+
         if ($reviewing) {
             $options = $this->get_review_options();
         } else {
             $options = $this->get_render_options($id);
         }
         if ($thispageurl) {
-            $this->quiz->thispageurl = $thispageurl;
+            $this->quiz->thispageurl = str_replace($CFG->wwwroot, '', $thispageurl->out(false));
         } else {
             unset($thispageurl);
         }
@@ -992,7 +994,7 @@ class quiz_attempt extends quiz {
             }
         }
 
-        // Add a fragment to scroll down ot the question.
+        // Add a fragment to scroll down to the question.
         if ($questionid) {
             if ($questionid == reset($this->pagequestionids[$page])) {
                 // First question on page, go to top.

mod/quiz/editlib.php

@@ -392,7 +392,7 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true,
 
     $pageopen = false;
 
-    $returnurl = $pageurl->out();
+    $returnurl = str_replace($CFG->wwwroot, '', $pageurl->out(false));
     $questiontotalcount = count($order);
 
     foreach ($order as $i => $qnum) {
@@ -436,9 +436,11 @@ function quiz_print_question_list($quiz, $pageurl, $allowdelete = true,
 
             if ($qnum != 0) {
                 $question = $questions[$qnum];
-                $questionparams = array('returnurl' => $returnurl,
-                        'cmid' => $quiz->cmid, 'id' => $question->id);
-                $questionurl = new moodle_url("$CFG->wwwroot/question/question.php",
+                $questionparams = array(
+                        'returnurl' => $returnurl,
+                        'cmid' => $quiz->cmid,
+                        'id' => $question->id);
+                $questionurl = new moodle_url('/question/question.php',
                         $questionparams);
                 $questioncount++;
                 //this is an actual question
@@ -644,7 +646,7 @@ function quiz_print_pagecontrols($quiz, $pageurl, $page, $hasattempts) {
     $returnurladdtoquiz = new moodle_url($pageurl, array('addonpage' => $page));
 
     // Print a button linking to the choose question type page.
-    $returnurladdtoquiz = str_replace($CFG->wwwroot . '/', '', $returnurladdtoquiz->out(false));
+    $returnurladdtoquiz = str_replace($CFG->wwwroot, '', $returnurladdtoquiz->out(false));
     $newquestionparams = array('returnurl' => $returnurladdtoquiz,
             'cmid' => $quiz->cmid, 'appendqnumstring' => 'addquestion');
     create_new_question_button($defaultcategory->id, $newquestionparams, get_string('addaquestion', 'quiz'),

question/addquestion.php

@@ -82,8 +82,8 @@ $PAGE->set_url('/question/addquestion.php', $hiddenparams);
 $chooseqtype = get_string('chooseqtypetoadd', 'question');
 $PAGE->set_heading($COURSE->fullname);
 if ($cm !== null) {
-    if (stripos($returnurl, "$CFG->wwwroot/mod/{$cm->modname}/view.php")!== 0) {
-        //don't need this link if returnurl returns to view.php
+    // Nasty hack, but we don't want this link if returnurl returns to view.php
+    if (stripos($returnurl, "/mod/{$cm->modname}/view.php")!== 0) {
         $PAGE->navbar->add(get_string('editinga', 'moodle', get_string('modulename', $cm->modname)),$returnurl);
     }
     $PAGE->navbar->add($chooseqtype);

question/editlib.php

@@ -801,8 +801,7 @@ class question_bank_view {
         }
 
         // Create the url of the new question page to forward to.
-        $returnurl = $pageurl->out(false);
-        $returnurl = str_replace($CFG->wwwroot . '/', '', $returnurl);
+        $returnurl = str_replace($CFG->wwwroot, '', $pageurl->out(false));
         $this->editquestionurl = new moodle_url('/question/question.php',
                 array('returnurl' => $returnurl));
         if ($cm !== null){

question/question.php

@@ -59,12 +59,15 @@ if ($inpopup !== 0) {
 }
 $PAGE->set_url($url);
 
-if ($originalreturnurl && $wizardnow == '') {
-    $returnurl = $CFG->wwwroot . '/' . $originalreturnurl;
-} else if ($originalreturnurl && $wizardnow !== ''){
-    $returnurl = $originalreturnurl;
-}else {
-    $returnurl = "{$CFG->wwwroot}/question/edit.php?courseid={$COURSE->id}";
+if ($originalreturnurl) {
+    if (strpos($originalreturnurl, '/') !== 0) {
+        throw new coding_exception("returnurl must be a local URL starting with '/'. $originalreturnurl was given.");
+    }
+    $returnurl = new moodle_url($originalreturnurl);
+} else if ($cmid) {
+    $returnurl = new moodle_url('/question/edit.php', array('cmid' => $cmid));
+} else {
+    $returnurl = new moodle_url('/question/edit.php', array('courseid' => $courseid));
 }
 
 if ($movecontext && !$id){
@@ -236,10 +239,7 @@ if ($mform->is_cancelled()){
     }
 
     /// Ensure we redirect back to the category the question is being saved into.
-    $returnurl = new moodle_url($returnurl);
     $returnurl->param('category', $fromform->category);
-    // TODO: it is sloppy to pass arounf full URLs through page parameters and some servers do not like that
-    $returnurl = $returnurl->out(false);
 
     /// Call the appropriate method.
     if ($movecontext) {
@@ -247,14 +247,14 @@ if ($mform->is_cancelled()){
         $tocontext = get_context_instance_by_id($tocontextid);
         require_capability('moodle/question:add', $tocontext);
         if (get_filesdir_from_context($categorycontext) != get_filesdir_from_context($tocontext)){
-            $movecontexturl  = new moodle_url('/question/contextmoveq.php',
-                                            array('returnurl' => $returnurl,
-                                                    'ids'=>$question->id,
-                                                    'tocatid'=> $tocatid));
+            $movecontexturl  = new moodle_url('/question/contextmoveq.php', array(
+                    'returnurl' => str_replace($CFG->wwwroot, '', $returnurl->out(false)),
+                    'ids' => $question->id,
+                    'tocatid' => $tocatid));
             if ($cmid){
                 $movecontexturl->param('cmid', $cmid);
             } else {
-                $movecontexturl->param('courseid', $COURSE->id);
+                $movecontexturl->param('courseid', $courseid);
             }
             redirect($movecontexturl);
         }
@@ -272,21 +272,25 @@ if ($mform->is_cancelled()){
             echo $OUTPUT->notification(get_string('changessaved'), '');
             close_window(3);
         } else {
-            $nexturl = new moodle_url($returnurl);
-            $nexturl->param('lastchanged', $question->id);
-            if($appendqnumstring) {
-                $nexturl->params(array($appendqnumstring=>($question->id), "sesskey"=>sesskey(), "cmid"=>$cmid));
+            $returnurl->param('lastchanged', $question->id);
+            if ($appendqnumstring) {
+                $returnurl->param($appendqnumstring, $question->id);
+                $returnurl->param('sesskey', sesskey());
+                $returnurl->param('cmid', $cmid);
             }
-            redirect($nexturl);
+            redirect($returnurl);
         }
     } else {
-        $nexturlparams = array('returnurl'=>$returnurl, 'appendqnumstring'=>$appendqnumstring);
+        $nexturlparams = array(
+                'returnurl' => $originalreturnurl,
+                'appendqnumstring' => $appendqnumstring);
         if (isset($fromform->nextpageparam) && is_array($fromform->nextpageparam)){
-            $nexturlparams += $fromform->nextpageparam;//useful for passing data to the next page which is not saved in the database
+            //useful for passing data to the next page which is not saved in the database.
+            $nexturlparams += $fromform->nextpageparam;
         }
         $nexturlparams['id'] = $question->id;
         $nexturlparams['wizardnow'] = $fromform->wizard;
-        $nexturl = new moodle_url('question.php', $nexturlparams);
+        $nexturl = new moodle_url('/question/question.php', $nexturlparams);
         if ($cmid){
             $nexturl->param('cmid', $cmid);
         } else {

question/type/questiontype.php

@@ -1063,30 +1063,29 @@ class default_questiontype {
         }
 
     /// Work out the right URL.
-        $linkurl = '/question/question.php?id=' . $question->id;
+        $url = new moodle_url('/question/question.php', array('id' => $question->id));
         if (!empty($cmoptions->cmid)) {
-            $linkurl .= '&cmid=' . $cmoptions->cmid;
+            $url->param('cmid', $cmoptions->cmid);
         } else if (!empty($cmoptions->course)) {
-            $linkurl .= '&courseid=' . $cmoptions->course;
+            $url->param('courseid', $cmoptions->course);
         } else {
             print_error('missingcourseorcmidtolink', 'question');
         }
 
-    /// Work out the contents of the link.
-        $stredit = get_string('edit');
-        $linktext = '<img src="' . $OUTPUT->pix_url('t/edit') . '" alt="' . $stredit . '" />';
+        $icon = new pix_icon('t/edit', get_string('edit'));
 
+        $action = null;
         if (!empty($cmoptions->thispageurl)) {
-        /// The module allow editing in the same window, print an ordinary link.
-            return '<a href="' . $CFG->wwwroot . $linkurl . '&amp;returnurl=' .
-                    urlencode($cmoptions->thispageurl . '#q' . $question->id) .
-                    '" title="' . $stredit . '">' . $linktext . '</a>';
+            // The module allow editing in the same window, print an ordinary
+            // link with a returnurl.
+            $url->param('returnurl', $cmoptions->thispageurl);
         } else {
-        /// We have to edit in a pop-up.
-            $link = new moodle_url($linkurl . '&inpopup=1');
+            // We have to edit in a pop-up.
+            $url->param('inpopup', 1);
             $action = new popup_action('click', $link, 'editquestion');
-            return $OUTPUT->action_link($link, $linktext, $action ,array('title'=>$stredit));
         }
+
+        return $OUTPUT->action_icon($url, $icon, $action);
     }
 
     /**